Mir

Code review comment for lp:~andreas-pokorny/mir/input-configuration-api

Andreas Pokorny (andreas-pokorny) wrote :

> + virtual void apply_configuration(PointerConfiguration const&) = 0;
> + virtual void apply_configuration(TouchPadConfiguration const&) = 0;
>
> I find it easier to read code when the functions are not overloaded in this
> way, but have unique names instead, e.g.:
>
> apply_pointer_configuration()
> apply_touchpad_configuration()
>
> I think if functions are performing different operations they should be named
> differently. We should only use overloading for functions that perform the
> same operation but need different arguments.

But in both cases I only apply that configuration. I mean this is the same operation just different arguments.

> [...]
> + void scroll_mode(MirTouchPadScrollMode scroll_mode);
> + MirTouchPadScrollMode scroll_mode() const;
> + void scroll_mode(int scroll_button);
>
> ... but this is just confusing. The first overload scroll_mode() sets the
> scroll mode but the second overload adds to it (and changes the button scroll
> mode), so these are different operations and the functions should be named
> differently.

Ok I aggree this is confusing..

« Back to merge proposal