The Inbox: Tools-ct.945.mcz
Locked
 
|
Christoph Thiede uploaded a new version of Tools to project The Inbox:
http://source.squeak.org/inbox/Tools-ct.945.mcz ==================== Summary ==================== Name: Tools-ct.945 Author: ct Time: 24 February 2020, 1:56:51.392323 pm UUID: cc577466-ca48-484b-9454-11ae7ffde0b2 Ancestors: Tools-mt.940 Fixes a bug/unnecessary limitation in VersionsBrowser class >> #browseMethod: that raised an error when browsing a method that had been removed from the system. You can also reproduce it via: thisContext method browse "on a fresh image". This commit replaces Tools-ct.944, which has been refactored again to reuse the return carat according to Kent Beck ("Format conditionals so their value is used where it clearly expresses the intent of the method"). Thanks to Chris for the reminder! =============== Diff against Tools-mt.940 =============== Item was changed: ----- Method: VersionsBrowser class>>browseMethod: (in category 'instance creation') ----- browseMethod: aCompiledMethod + ^ (self browseVersionsOf: aCompiledMethod) + ifNotNil: [:browser | + browser selectMethod: aCompiledMethod]; - ^ (self browseVersionsForClass: aCompiledMethod methodClass selector: aCompiledMethod selector) - selectMethod: aCompiledMethod; yourself! Item was added: + ----- Method: VersionsBrowser class>>browseVersionsOf: (in category 'instance creation') ----- + browseVersionsOf: aCompiledMethod + + | methodClass methodSelector | + methodClass := aCompiledMethod methodClass. + methodSelector := aCompiledMethod selector. + ^ self + browseVersionsOf: aCompiledMethod + class: methodClass + meta: methodClass isMeta + category: (methodClass organization categoryOfElement: methodSelector) + selector: methodSelector! |
Re: The Inbox: Tools-ct.945.mcz
|
|
Hi Christoph,
I still feel like using #ifNil:, #ifNotNil: in a cascade is not right. In this case, having #yourself inside the #ifNotNil: block looks more natural to me: ^(self browseVersionsOf: aCompiledMethod) ifNotNil: [:browser | browser selectMethod: aCompiledMethod; yourself ] Levente On Mon, 24 Feb 2020, [hidden email] wrote: > Christoph Thiede uploaded a new version of Tools to project The Inbox: > http://source.squeak.org/inbox/Tools-ct.945.mcz > > ==================== Summary ==================== > > Name: Tools-ct.945 > Author: ct > Time: 24 February 2020, 1:56:51.392323 pm > UUID: cc577466-ca48-484b-9454-11ae7ffde0b2 > Ancestors: Tools-mt.940 > > Fixes a bug/unnecessary limitation in VersionsBrowser class >> #browseMethod: that raised an error when browsing a method that had been removed from the system. > > You can also reproduce it via: thisContext method browse "on a fresh image". > > This commit replaces Tools-ct.944, which has been refactored again to reuse the return carat according to Kent Beck ("Format conditionals so their value is used where it clearly expresses the intent of the method"). Thanks to Chris for the reminder! > > =============== Diff against Tools-mt.940 =============== > > Item was changed: > ----- Method: VersionsBrowser class>>browseMethod: (in category 'instance creation') ----- > browseMethod: aCompiledMethod > > + ^ (self browseVersionsOf: aCompiledMethod) > + ifNotNil: [:browser | > + browser selectMethod: aCompiledMethod]; > - ^ (self browseVersionsForClass: aCompiledMethod methodClass selector: aCompiledMethod selector) > - selectMethod: aCompiledMethod; > yourself! > > Item was added: > + ----- Method: VersionsBrowser class>>browseVersionsOf: (in category 'instance creation') ----- > + browseVersionsOf: aCompiledMethod > + > + | methodClass methodSelector | > + methodClass := aCompiledMethod methodClass. > + methodSelector := aCompiledMethod selector. > + ^ self > + browseVersionsOf: aCompiledMethod > + class: methodClass > + meta: methodClass isMeta > + category: (methodClass organization categoryOfElement: methodSelector) > + selector: methodSelector! |
Re: The Inbox: Tools-ct.945.mcz
|
> I still feel like using #ifNil:, #ifNotNil: in a cascade is not right. Me too. It's hard to read. Best, Marcel
|
Re: The Inbox: Tools-ct.945.mcz
|
|
Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 25. Februar 2020 15:13:42 An: John Pfersich via Squeak-dev Betreff: Re: [squeak-dev] The Inbox: Tools-ct.945.mcz
> I still feel like using #ifNil:, #ifNotNil: in a cascade is not right.
Me too. It's hard to read.
Best,
Marcel
Carpe Squeak!
|
Re: The Inbox: Tools-ct.945.mcz
|
|
Hi Christoph, Hi All, On Feb 25, 2020, at 9:33 AM, Thiede, Christoph <[hidden email]> wrote:
All of ifTrue: ifFalse: and ifNotNil: evaluate to false if their receiver is that which doesn’t select the block. Not knowing this is illiteracy. The definitions are there to be read (and there’s only six of them ferchrissakes). Please let’s not go the gcc way and through passive aggressive warnings force every thing to be parenthesized and to not use ifTrue: ifFalse: and ifNotNil:, and to have to say [nil] instead of []. Conciseness is a value in itself and the basics of Smalltalk literacy are very little effort to attain. I remember with some horror in 1995 finding lots of code in the VisualWorks vm that looked like if (EXPR == true) ... Please, no.
|
Re: The Inbox: Tools-ct.945.mcz
|
|
> All of ifTrue: ifFalse: and ifNotNil: evaluate to false
You mean nil. Alright! :-)
Best, Christoph Von: Squeak-dev <[hidden email]> im Auftrag von Eliot Miranda <[hidden email]>
Gesendet: Dienstag, 25. Februar 2020 18:49:52 An: The general-purpose Squeak developers list Betreff: Re: [squeak-dev] The Inbox: Tools-ct.945.mcz Hi Christoph, Hi All,
On Feb 25, 2020, at 9:33 AM, Thiede, Christoph <[hidden email]> wrote:
All of ifTrue: ifFalse: and ifNotNil: evaluate to false if their receiver is that which doesn’t select the block. Not knowing this is illiteracy. The definitions are there to be read (and there’s only six of them ferchrissakes). Please let’s not go
the gcc way and through passive aggressive warnings force every thing to be parenthesized and to not use ifTrue: ifFalse: and ifNotNil:, and to have to say [nil] instead of []. Conciseness is a value in itself and the basics of Smalltalk literacy are very
little effort to attain.
I remember with some horror in 1995 finding lots of code in the VisualWorks vm that looked like
if (EXPR == true) ...
Please, no.
Carpe Squeak!
|
Re: The Inbox: Tools-ct.945.mcz
|
|
On Feb 25, 2020, at 9:57 AM, Thiede, Christoph <[hidden email]> wrote:
oops :-(
|
Re: The Inbox: Tools-ct.945.mcz
|
|
In reply to this post by Eliot Miranda-2
Agree with Eliot on all except this one:
because empty blocks are ambiguous with incomplete code. That's why, if nil is "supposed" to be the return value, I always explicitly specify it, as I would any other value, and not an empty block. If the return value isn't consumed, and I simply wish to "do nothing", then I'll write a short comment like, "do nothing" to express the that intention. Leaving it empty could cause future readers to wonder whether the code was ever finished, or not. It fits in with why we format code across multiple, indented lines -- it's not the most concise, but it is more literate. Literacy is about communicating well with readers, not necessarily always writing the most-concise thing possible... |
Re: The Inbox: Tools-ct.945.mcz
|
|
On Tue, 25 Feb 2020, Chris Muller wrote:
> Agree with Eliot on all except this one: > > and to have to say [nil] instead of []. > > > because empty blocks are ambiguous with incomplete code. That's why, if nil is "supposed" to be the return value, I always explicitly specify it, as I would any other value, and not an empty block. > > If the return value isn't consumed, and I simply wish to "do nothing", then I'll write a short comment like, "do nothing" to express the that intention. Leaving it empty could cause future readers to wonder whether the code was ever finished, or not. In the context of the Trunk, that assumption would be wrong. Incomplete code with no documentation has no place in there. Levente > > It fits in with why we format code across multiple, indented lines -- it's not the most concise, but it is more literate. Literacy is about communicating well with readers, not necessarily always writing the most-concise thing possible... > > > > |
Re: The Inbox: Tools-ct.945.mcz
|
|
On Wed, Feb 26, 2020 at 5:20 AM Levente Uzonyi <[hidden email]> wrote: On Tue, 25 Feb 2020, Chris Muller wrote: I like your commitment to quality! :) But, it ignores the previous sentences about expressivity of intent, standing on its own in the code. Future readers might be unaware of trunk's quality-control measures, which evolved over time. We still write sends to #yourself to reveal intention, even though it's less concise.. I'm game enough myself to read empty blocks, but I don't care to write them, because I think there's a slight disadvantage for outside readers, regardless of literacy, in leaving them ambiguous. - Chris |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |