Hi Christoph,

On Mon, 30 Mar 2020, Thiede, Christoph wrote:

> 
> Hi Nicolas,
> 
> 
> > There is no other sender of doWhileFalse: doWhileTrue: and my advice would be to deprecate them. YAGNI.
> 
> 
> So my image actually contains around two dozen senders of #doWhileTrue: and #doWhileFalse: (Squot, Pheno and some of my own code). As mentioned somewhere else in the past, I actually like them and find them very useful.
> 
> In my opinion, many senders of #whileTrue and #whileFalse should be refactored to use #doWhileTrue:/#doWhileFalse:. Here is a random example:
> 
> [newPlace := self getNewPlace.
> dir := ServerFile new fullPath: newPlace.
> (dir includesKey: dir fileName)] whileTrue.
> 
> I find that's ugly and counter-intuitive! I would rewrite this in the following way:
> 
> [newPlace := self getNewPlace.
> dir := ServerFile new fullPath: newPlace]
>     doWhileTrue: [dir includesKey: dir fileName].

I brought up #doWhileTrue: recently as methods that should be removed[1]. 
And yeah, I would never ever use it in a method. Not just because it's 
redundant, as you pointed out above, but because the name is confusing: do 
what while what's true?

> 
> The latter just reads easier. blockReturns in multi-statement blocks are not really intuitive.
> In the first version, when scanning the method quickly, I read "okay, something with places and directories is done, while true ...," I wonder "what has to be true?" and have to look back to the end of the block and find the
> beginning of the last statement.
> But in the second version, I read "okay, something with places and directories is done, while that directory matches this specific condition." This version directly draws my attention to the important condition.

Couldn't disagree more with that. It's exactly the other way around. If 
you know the language, you know that a block returns its last statement's 
value, so you know what has to be true in case of #whileTrue. But the 
language tells you nothing about how #doWhileTrure: works. The name is 
just confusing (see above).


Levente

> 
> Happy to hear your counterarguments :-)
> 
> Best,
> Christoph
> 
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von commits at source.squeak.org <commits at source.squeak.org>
> Gesendet: Montag, 30. März 2020 12:51 Uhr
> An: squeak-dev at lists.squeakfoundation.org; packages at lists.squeakfoundation.org
> Betreff: [squeak-dev] The Trunk: System-nice.1149.mcz  
> Nicolas Cellier uploaded a new version of System to project The Trunk:
> http://source.squeak.org/trunk/System-nice.1149.mcz
> 
> ==================== Summary ====================
> 
> Name: System-nice.1149
> Author: nice
> Time: 30 March 2020, 12:50:26.196366 pm
> UUID: ece53b0b-24d2-4d4b-bc91-6794a661f886
> Ancestors: System-ul.1148
> 
> avoid neuron storming [...] doWhileTrue: true, it just means [...] repeat
> 
> There is no other sender of doWhileFalse: doWhileTrue: and my advice would be to deprecate them. YAGNI.
> 
> =============== Diff against System-ul.1148 ===============
> 
> Item was changed:
>   ----- Method: MOFile>>searchByHash: (in category 'experimental') -----
>   searchByHash: aString
>          | hashValue nstr index incr key |
>          hashValue :=  self hashPjw: aString.
>          incr := 1 + (hashValue \\ (hashTableSize -2)).
>          index := (hashValue \\ hashTableSize) .
>          [        nstr := (hashTable at: index +1 ).
>                  nstr = 0 ifTrue: [^nil].
>                  key := self originalString: nstr.
>                  key = aString ifTrue: [^self translatedString: nstr].
>                  index >= (hashTableSize - incr)
>                                  ifTrue: [index := index - (hashTableSize - incr)  ]
>                                  ifFalse:[index := index + incr].       
> +        ] repeat!
> -        ] doWhileTrue: true.!
> 
> 
> 
>