[squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
Thiede, Christoph
Christoph.Thiede at student.hpi.uni-potsdam.de
Fri Oct 25 17:25:25 UTC 2019
Well, should I commit again, or is it easier for the merger to align the code style?
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 15. Oktober 2019 15:23:54
An: gettimothy via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
Hi Christoph. :-)
> I like it as a good readable shorthand.
That's debatable. ;-) "#contents" is a Symbol. "[:topic | topic contents]" is a block with message sends. I find the latter more readable because it reveals the message sends. You can debug it, put a "self halt" in it, etc.
> self defaultSubtopic ifNil: [''] ifNotNil: #contents
I don't like the asymmetry between block and symbol. So, in this case, I prefer blocks over that symbol shorthand for a second reason.
Best,
Marcel
Am 14.10.2019 11:55:39 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
Yes, you're right :)
You find the use of Symbol>>#value: "too meta"? I like it as a good readable shorthand.
Best,
Christoph
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Tobias Pape <Das.Linux at gmx.de>
Gesendet: Montag, 14. Oktober 2019 09:16:21
An: squeak-dev at lists.squeakfoundation.org
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
Hi
I'd like to propose a little change:
> On 14.10.2019, at 00:34, commits at source.squeak.org wrote:
>
> ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
> contents
> "A book has no contents. Only its pages do."
>
> + ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!
^ self defaultSubtopic
ifNotNil: [:topic | topic contents]
ifNil: ['']
This changes the order so that the non-exceptional case is first. It also avoids the meta-level.
> - ^ ''!
>
> Item was added:
> + ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
> + defaultSubtopic
> +
> + ^ self helpClass defaultPageKey
> + ifNotNil: [:key |
> + self subtopics
> + detect: [:topic | topic key = key]
> + ifNone: [nil]]
> + ifNil: [nil]!
I'd leave out the 'ifNil: [nil]' case; it's implied when sending ifNotNil:.
Best regards
-Tobias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191025/9d03f9d1/attachment-0001.html>
More information about the Squeak-dev
mailing list
|