[vlc-devel] commit: New C++ binding for libvlc. ( R?mi Duraffort )

[Pierre d'Herbemont]

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.
>>
>> Like:
>> 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);

Please dont.


> I also think that the wrapper classes should inline (much) more
> methods; now they're convenient but add entirely avoidable call
> overhead.


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  
> aren't
> 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,  
> but
> should nonetheless be fixed.

More than agreed.
>


Pierre.