<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p>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).<br>
</p>
<p><br>
</p>
<p>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.</p>
<p><br>
</p>
<p>Best,</p>
<p>Christoph</p>
<div id="Signature">
<div name="divtagdefaultwrapper" style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:; margin:0">
<div><font size="2" color="#808080"></font></div>
</div>
</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>Von:</b> Squeak-dev <squeak-dev-bounces@lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel<br>
<b>Gesendet:</b> Donnerstag, 15. August 2019 10:26:00<br>
<b>An:</b> gettimothy via Squeak-dev<br>
<b>Betreff:</b> Re: [squeak-dev] The Inbox: Graphics-ct.410.mcz</font>
<div> </div>
</div>
<div>
<div id="__MailbirdStyleContent" style="font-size: 10pt;font-family: Arial;color: #000000">
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?
<div><br>
</div>
<div>However, Tobias is right that there are only 15+74 uses of #caseOf:(otherwise:) in the image. We should review those carefully. </div>
<div><br>
</div>
<div>Best,</div>
<div>Marcel</div>
<div class="mb_sig"></div>
<blockquote class="history_container" type="cite" style="border-left-style:solid;border-width:1px; margin-top:20px; margin-left:0px;padding-left:10px;">
<p style="color: #AAAAAA; margin-top: 10px;">Am 15.08.2019 02:16:53 schrieb Tobias Pape <das.linux@gmx.de>:</p>
<div style="font-family:Arial,Helvetica,sans-serif"><br>
> On 14.08.2019, at 23:52, commits@source.squeak.org wrote:<br>
> <br>
> A new version of Graphics was added to project The Inbox:<br>
> http://source.squeak.org/inbox/Graphics-ct.410.mcz<br>
> <br>
> ==================== Summary ====================<br>
> <br>
> Name: Graphics-ct.410<br>
> Author: ct<br>
> Time: 14 August 2019, 11:52:24.4754 pm<br>
> UUID: 008f8bb1-9963-1e46-8b29-1f1e2aa3235b<br>
> Ancestors: Graphics-nice.409<br>
> <br>
> Little refactoring to HSV conversion<br>
> <br>
> 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?<br>
> <br>
<br>
-1.<br>
<br>
I do not hold that caseOf is more beautiful. It may be a matter of taste, here.<br>
But often it's an instance of forgotten polymorphy, so I'm against promoting it outside "important" uses, eg in Parser or so.<br>
<br>
I see that this is a lot number-dancing and maybe not even an important method, so it might not matter much.
<br>
Yet I don't see benefit in the #caseOf variant.<br>
<br>
Best regards<br>
-Tobias<br>
<br>
> =============== Diff against Graphics-nice.409 ===============<br>
> <br>
> Item was changed:<br>
> ----- Method: Color>>setHue:saturation:brightness: (in category 'private') -----<br>
> setHue: hue saturation: saturation brightness: brightness<br>
> "Initialize this color to the given hue, saturation, and brightness. See the comment in the instance creation method for details."<br>
> <br>
> | s v hf i f p q t | <br>
> s := (saturation asFloat max: 0.0) min: 1.0.<br>
> v := (brightness asFloat max: 0.0) min: 1.0.<br>
> <br>
> "zero saturation yields gray with the given brightness"<br>
> s = 0.0 ifTrue: [ ^ self setRed: v green: v blue: v ].<br>
> <br>
> hf := hue asFloat.<br>
> (hf < 0.0="" or:="" [hf="">= 360.0])<br>
> ifTrue: [hf := hf \\ 360].<br>
> hf := hf / 60.0.<br>
> i := hf asInteger. "integer part of hue"<br>
> f := hf fractionPart. "fractional part of hue"<br>
> p := (1.0 - s) * v.<br>
> q := (1.0 - (s * f)) * v.<br>
> t := (1.0 - (s * (1.0 - f))) * v.<br>
> <br>
> + ^ i caseOf: {<br>
> + [0] -> [ self setRed: v green: t blue: p ].<br>
> + [1] -> [ self setRed: q green: v blue: p ].<br>
> + [2] -> [ self setRed: p green: v blue: t ].<br>
> + [3] -> [ self setRed: p green: q blue: v ].<br>
> + [4] -> [ self setRed: t green: p blue: v ].<br>
> + [5] -> [ self setRed: v green: p blue: q ]. }!<br>
> - 0 = i ifTrue: [ ^ self setRed: v green: t blue: p ].<br>
> - 1 = i ifTrue: [ ^ self setRed: q green: v blue: p ].<br>
> - 2 = i ifTrue: [ ^ self setRed: p green: v blue: t ].<br>
> - 3 = i ifTrue: [ ^ self setRed: p green: q blue: v ].<br>
> - 4 = i ifTrue: [ ^ self setRed: t green: p blue: v ].<br>
> - 5 = i ifTrue: [ ^ self setRed: v green: p blue: q ].<br>
> - <br>
> - self error: 'implementation error'.<br>
> - !<br>
> <br>
> <br>
<br>
<br>
<br>
</div>
</blockquote>
</div>
</div>
</body>
</html>