Hi Nicolas,


I don't see your point of #doWhileTrue: being not idiomatic. What exactly do you mean by that except for "unfamiliar"?


However, I agree that the names are confusing, compared to #whileTrue: etc.

+1 for naming them #repeatWhile: and #repeatUntil: instead!


> But that still means that we have two ways to express exactly the same thing, which by first principle should not be

> (economy of implementation, of tests, of documentation, etc... small is beautiful !).

Well, that's another a question of semantics, again. :) Remember my examples about #ifNotNil:/#==> etc.? Convenience methods support ease of writing and reading.

> Really, not kidding?

The example is taken from BookMorph >> #saveOnUrlPage:. I did not read it in context, too ^^

> Is the following better?
>
> [newPlace := self getNewPlace.
> file:= ServerFile new fullPath: newPlace.
> file exists] whileTrue

In my opinion, #repeatUntil: would still be better :)

> Note that I did not add non local block return in the middle of the block...
> It was in the original method, doWhileTrue did not solve it.

I was referring to blockReturn in the bytecode sense. Here is another example of confusing blockReturns:

(foo satisfiesSomeCondition or: [|baz|
    baz := bar asBaz.
    foo cacheDuring: [
        baz satisfiesSomeOtherCondition]])
            ifTrue: [...]

Hard to follow the control flow, isn't it? If I have the choice, I avoid these statements and use blocks,
either) to pass a piece of code to another object and compose control flows,
or) to explicitly pass a value to be evaluated lazily.
Both at once can be confusing. I think there is a good reason why there is no syntax for blockReturn like we have one for methodReturn (^).

Best,
Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
Gesendet: Montag, 30. März 2020 20:56:43
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] #doWhileTrue: (was: The Trunk: System-nice.1149.mcz)



Le lun. 30 mars 2020 à 19:58, Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de<mailto:Christoph.Thiede at student.hpi.uni-potsdam.de>> a écrit :

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 first thing that upsets me is that they are not idiomatic. #whileTrue: and in a lesser way #whileTrue are.
The second thing that upsets me is that the names are REALLY too close to #whileTrue:
So at first, I was confused by the fact that they looked like synonyms.
Then I wondered exactly the same, what the hell has to be true?
Especially when the sole sender in trunk is [] doWhileTrue: true ;)

IMO, they might be better spelled #repeatWhile: and #repeatUntil:
But that still means that we have two ways to express exactly the same thing, which by first principle should not be
(economy of implementation, of tests, of documentation, etc... small is beautiful !).

We might discuss which is more readable, maybe

[newPlace := self getNewPlace.
dir := ServerFile new fullPath: newPlace]
    repeatWhile: [dir includesKey: dir fileName].

is more revealing than:

[newPlace := self getNewPlace.
dir := ServerFile new fullPath: newPlace.
(dir includesKey: dir fileName)] whileTrue.

IMO, it's a biased example, because the whole snippet is un-comprehensible.
So a ServerFile behaves like a ServerDirectory (from which it inherits...), oh nice (???).
And a ServerFile which would includes some item with same name as itself would mean that the file already exists...
Really, not kidding?

So IMO that's what makes the snippet illegible, not whileTrue.
Is the following better?

[newPlace := self getNewPlace.
file:= ServerFile new fullPath: newPlace.
file exists] whileTrue

Anyway, the second one is there for such a long time that you can't expunge it that easily. It's idiomatic...

The latter just reads easier. blockReturns in multi-statement blocks are not really intuitive.

Note that I did not add non local block return in the middle of the block...
It was in the original method, doWhileTrue did not solve it.

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<mailto:squeak-dev-bounces at lists.squeakfoundation.org>> im Auftrag von commits at source.squeak.org<mailto:commits at source.squeak.org> <commits at source.squeak.org<mailto:commits at source.squeak.org>>
Gesendet: Montag, 30. März 2020 12:51 Uhr
An: squeak-dev at lists.squeakfoundation.org<mailto:squeak-dev at lists.squeakfoundation.org>; packages at lists.squeakfoundation.org<mailto: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/07a33b55/attachment-0001.html>