[squeak-dev] The Inbox: Graphics-ct.410.mcz

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Thu Aug 15 09:10:54 UTC 2019


What should be the general problem with #caseOf:? Just in this example, I think #caseOf: provides a better alternative to an amount of logic. #caseOf: describes a more data-driven approach to express repeating logic. In particular when you don't have a default case, you won't need to explicitly write a check for that, but #caseOf: will handle that for you (so I think in the current example, a caseError just has the right semantic. Apart from that I think errors that are never ever expected to be raised are some kind of code smell).


Of course, excessive use of #caseOf: might indicate the lack of an inheritance tree, but I don't think this is specific to #caseOf:, the same smell can be written with many conditionals as well.


Best,

Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Donnerstag, 15. August 2019 10:26:00
An: gettimothy via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Graphics-ct.410.mcz

Hmm... looking at PNMReadWriter >> #nextImage or MimeConverter class >> #forEncoding:, the use in Color looks similar and valid. What about the otherwise-case and the former "implementation error"? A case error would have a different meaning?

However, Tobias is right that there are only 15+74 uses of #caseOf:(otherwise:) in the image. We should review those carefully.

Best,
Marcel

Am 15.08.2019 02:16:53 schrieb Tobias Pape <das.linux at gmx.de>:

> On 14.08.2019, at 23:52, commits at source.squeak.org wrote:
>
> A new version of Graphics was added to project The Inbox:
> http://source.squeak.org/inbox/Graphics-ct.410.mcz
>
> ==================== Summary ====================
>
> Name: Graphics-ct.410
> Author: ct
> Time: 14 August 2019, 11:52:24.4754 pm
> UUID: 008f8bb1-9963-1e46-8b29-1f1e2aa3235b
> Ancestors: Graphics-nice.409
>
> Little refactoring to HSV conversion
>
> Afaik #caseOf: is compiler optimized, so this is not only more beautiful but also at least at efficient as the old conditional chain, isn't it?
>

-1.

I do not hold that caseOf is more beautiful. It may be a matter of taste, here.
But often it's an instance of forgotten polymorphy, so I'm against promoting it outside "important" uses, eg in Parser or so.

I see that this is a lot number-dancing and maybe not even an important method, so it might not matter much.
Yet I don't see benefit in the #caseOf variant.

Best regards
-Tobias

> =============== Diff against Graphics-nice.409 ===============
>
> Item was changed:
> ----- Method: Color>>setHue:saturation:brightness: (in category 'private') -----
> setHue: hue saturation: saturation brightness: brightness
> "Initialize this color to the given hue, saturation, and brightness. See the comment in the instance creation method for details."
>
> | s v hf i f p q t |
> s := (saturation asFloat max: 0.0) min: 1.0.
> v := (brightness asFloat max: 0.0) min: 1.0.
>
> "zero saturation yields gray with the given brightness"
> s = 0.0 ifTrue: [ ^ self setRed: v green: v blue: v ].
>
> hf := hue asFloat.
> (hf < 0.0="" or:="" [hf="">= 360.0])
> ifTrue: [hf := hf \\ 360].
> hf := hf / 60.0.
> i := hf asInteger. "integer part of hue"
> f := hf fractionPart. "fractional part of hue"
> p := (1.0 - s) * v.
> q := (1.0 - (s * f)) * v.
> t := (1.0 - (s * (1.0 - f))) * v.
>
> + ^ i caseOf: {
> + [0] -> [ self setRed: v green: t blue: p ].
> + [1] -> [ self setRed: q green: v blue: p ].
> + [2] -> [ self setRed: p green: v blue: t ].
> + [3] -> [ self setRed: p green: q blue: v ].
> + [4] -> [ self setRed: t green: p blue: v ].
> + [5] -> [ self setRed: v green: p blue: q ]. }!
> - 0 = i ifTrue: [ ^ self setRed: v green: t blue: p ].
> - 1 = i ifTrue: [ ^ self setRed: q green: v blue: p ].
> - 2 = i ifTrue: [ ^ self setRed: p green: v blue: t ].
> - 3 = i ifTrue: [ ^ self setRed: p green: q blue: v ].
> - 4 = i ifTrue: [ ^ self setRed: t green: p blue: v ].
> - 5 = i ifTrue: [ ^ self setRed: v green: p blue: q ].
> -
> - self error: 'implementation error'.
> - !
>
>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20190815/fc7ffa92/attachment.html>


More information about the Squeak-dev mailing list