[Cuis-dev] Interesting thoughts about Color

[Phil B]

Juan,

On Sun, Apr 28, 2019 at 6:48 PM Juan Vuletich <juan at jvuletich.org> wrote:

> Hi Phil,
>
> On 4/28/2019 2:20 PM, Phil B via Cuis-dev wrote:
> > I was reading a thread going on over on pharo-dev about Color and
> > thought it was apropos for us.  Many of the more commonly used
> > instance methods (#lighter, #darker, #alpha: etc.) are named as if
> > they were accessors or actions which would most naturally be assumed
> > to modify the current color but actually create a new color.  If you
> > work with Color to any extent the current behavior is accepted as
> > normal since it is quasi-immutable, but this naming/behavior impedance
> > mismatch does seem pretty obvious when you think about it.  Perhaps we
> > should think about modifying these method names to better reflect what
> > they are actually doing? (and perhaps make Color more explicitly
> > immutable?)
>
> I think that `Color red darker` makes perfect sense. No one should think
> that it takes "red" and turns "all red" into "dark red". It should be
> understood as 'just something like red, but darker'.
>
>
Yes, but only if you're used to working with Color.  If you think about it
relative to other classes it's actually pretty weird: the normal Smalltalk
way of thinking would be that Color red returns a (new) red color, not a
singleton instance. (which I think is the crux of why the implementation
seems odd: it's neither a true singleton nor an immediate but has a bit of
feel from both)  If you had another (non-Color) object instance, it would
be reasonable to expect #darker to make just that instance darker, not all
instances or to return a new instance.  So yes, you get used to it if
you're used to Color... but it is rather different behavior from most of
the rest of the system.


> #alpha: is wrong, because it really looks like a setter, but Colors are
> (at least conceptually) immutable. You can not turn red into something
> else, in the same way you can not turn 7 into something else. #alpha:
> should be renamed #withAlpha:, so you can write `Color red withAlpha:
> 0.5` that does make sense. Same with #adjustBrightness: and
> #adjustSaturation:brightness:.
>
>
That was pretty much the point being made: rename the methods so it's clear
that they are returning a new color rather than modifying the current one.
(i.e. the issue is the naming, not the underlying implementation)


> WRT making Color immutable... All methods that change state are marked
> as 'private'. What else do you suggest?
>
>
Prefixing the setters with #private<whatever> would be my preference.

Anyway, I raised it because I thought it was an interesting and valid
point.  I'll leave it to you to decide what, if anything, makes sense to
change.


> Thanks,
>
> --
> Juan Vuletich
> www.cuis-smalltalk.org
> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
> https://github.com/jvuletich
> https://www.linkedin.com/in/juan-vuletich-75611b3
> @JuanVuletich
>
>
Thanks,
Phil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190428/c6da94e5/attachment-0001.html>