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].


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.

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.!


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