[vlc-devel] commit: New C++ binding for libvlc. ( R?mi Duraffort )
pdherbemont at gmail.com
Mon Jan 25 12:58:18 CET 2010
On Jan 25, 2010, at 9:48, jpd at videolan.org wrote:
> On Sat, Jan 23, 2010 at 05:10:36PM +0100, R?mi Duraffort wrote:
>>> Awesome! I think you should drop the get prefix for the getters.
>> MediaPlayer::time() for the getter
>> MediaPlayer::setTime() for the setter ?
> You don't even need a set prefix, overloading will take care of it:
> mtime_t MediaPlayer::time();
> void MediaPlayer::time(mtime_t);
> I also think that the wrapper classes should inline (much) more
> methods; now they're convenient but add entirely avoidable call
I dont think it's actually a good idea. You save nothing, and makes
headers unreadable. And it makes API even more a PITA.
> Otherwise, it's a reasonable start.
> As an aside, reading the docu-comments I see a lot of words that
> adding much value. Simple things like ``Get the agl handler associated
> with the media player'' describing what is clearly a getter method
> inside the media player class, are pretty much describing the obvious
> but what is an agl handler and why do I need it? The term hasn't been
> introduced and there's no reference where to find out more. This
> shouldn't be a tick-box item but should be written with a perspective
> to explain the prospective user not what it does but what it is there
> for and how to use it. This is a general problem with docu-comments,
> should nonetheless be fixed.
More than agreed.
More information about the vlc-devel