[vlc-devel] [PATCH 1/2] macosx: Add VLCDefaultValueSlider

Felix Paul K├╝hne fkuehne at videolan.org
Mon Dec 12 12:57:43 CET 2016


Hi Marvin,

Minor nitpicks :)

> On 11 Dec 2016, at 18:21, Marvin Scholz <epirat07 at gmail.com> wrote:
> 
> +- (void)setDefaultValue:(double)value;
> +
> +/**
> + Unsets the default value
> + 
> + \note Internally this sets the defaultValue to \c DBL_MAX
> + */
> +- (void)unsetDefaultValue;
> +
> +/**
> + Get the default value
> + 
> + \note It the returned value is \c DBL_MAX, there is no defaultValue set.
> + */
> +- (double)defaultValue;

Why not refactor as a @property with a documented how to re-set the default value?

> +- (void)setDefaultValue:(double)value;
> +- (void)unsetDefaultValue;
> +
> +- (double)defaultValue;

idem.

> +- (void)drawDefaultTickMarkWithFrame:(NSRect)rect
> +{
> +    [[NSColor grayColor] setFill];
> +    NSRectFill(rect);
> +}

How often is this method called? Does it make sense to cache the NSColor instance?

Best regards,

Felix



More information about the vlc-devel mailing list