[squeak-dev] The Trunk: Environments-cmm.37.mcz

Frank Shearar frank.shearar at gmail.com
Sat Dec 21 12:18:56 UTC 2013


On 20 December 2013 21:16, Chris Muller <asqueaker at gmail.com> wrote:
> On Fri, Dec 20, 2013 at 3:01 PM, Frank Shearar <frank.shearar at gmail.com> wrote:
>> On 20 December 2013 20:42,  <commits at source.squeak.org> wrote:
>>> Chris Muller uploaded a new version of Environments to project The Trunk:
>>> http://source.squeak.org/trunk/Environments-cmm.37.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Environments-cmm.37
>>> Author: cmm
>>> Time: 20 December 2013, 2:42:33.974 pm
>>> UUID: c2e36521-ef29-4aa3-9425-84793a2d007c
>>> Ancestors: Environments-nice.36
>>>
>>> Minor cleanups while trying to learn Environments.
>>>
>>> =============== Diff against Environments-nice.36 ===============
>>>
>>> Item was changed:
>>>   ----- Method: AddPrefixNamePolicy class>>prefix: (in category 'as yet unclassified') -----
>>> + prefix: aString
>>> +       ^ self new
>>> +                setPrefix: aString ;
>>> +                yourself!
>>> - prefix: aString
>>> -       ^ self basicNew initializeWithPrefix: aString!
>>
>> Why make the AddPrefixNamePolicy mutable? That seems like a bad idea.
>> Unless there's some seriously major gain to be had, we should _not_
>> add setters. The "basicNew initializeWithFoo:" idiom makes it clear
>> that "AddPrefixNamePolicy new" will not result in a properly
>> initialised object. It avoids giving people the impression that it's
>> OK to monkey with the object's state. It's also the closest we can get
>> to immutability, given that the only way we can initialise an object's
>> state correctly is by sending it messages. (I realise the idiom's not
>> in SBPP, but sometimes we can improve on the classics.)
>>
>> I like how you renamed lots of parameters to indicate the kind of
>> things they ought to contain ("aNamespace" -> "aDictionary"), but
>> please, no setters.
>
> Yeah, both things are simply documented best-practice conventions.
> It's best not to call basicNew except for serialization frameworks or
> otherwise with a good reason to, otherwise #initialize won't get
> called from the one place it should be called from.

My point is that I don't think you should send #new at all, because
that suggests that you could have an AddPrefixNamePolicy with no
prefix.

> So that requires changing the method name from #initializeWith...: to
> simply #set...:, which is Kent Becks "Constructor Parameter Method" on
> page 25 of his book.

I have a copy of SBPP (sadly not in the same country as me anymore),
which is why I specifically suggested that sometimes we can improve on
the classics :)

> Now, I've heard the suggestion to name these methods as "myPrefix:
> prefixString myNamespace: aDictionary", so that, should a developer
> ever think he wants to use them, it will look and read awkwardly in
> the calling code.
>
> But that relies on a convention for "privatizing" that no other
> private methods can afford.

As does that #new prefix: cascade.

> Therefore, nothing besides the categories/protocols are either needed,
> nor should be used, to indicate whether a method is ok to use from the
> outside.
>
> And, I confess, Beck says Constructor Parameter Methods should be
> categorized as 'private' but I trumped him because
> "initialize/release" is just as private (e.g., no one calls those from
> the outside) plus.. that's what it is for!  And, it co-locates it with
> #initialize where it belongs and easy to see all initialization under
> one protocol.
>
> Whew!

I almost entirely agree with you. Except that I think that calling
#basicNew and initialising within the #initializeWithFoo: methods is
superior because
* you don't need a setter (as in, you don't need a method that looks
and smells and tastes exactly like a classic
expose-my-state-for-mutation setter.
* #initializeWithFoo: tells you the ways you can construct a valid
instance, and if you want to construct an _invalid_ one you have to
work at it - actively subvert the API.
* #initializeWithFoo: tells you much louder exactly what it will do,
where #setFoo: (a) looks like a Java-style mutator and (b) suggests
that you can (partly!) _reinitialise_ an object. Worse, #setFoo: only
partially initialises the object, where #initializeWithFoo: completely
initialises an object.

So in the end we're arguing about two conventions that do more or less
the same thing. I don't see any _improvement_ in using #new, and in
fact see several (admittedly minor) downsides. So what's the point
then of "fixing" this?

(As I already said, I think the parameter renaming's a great idea.
It's just the #new setFoo: stuff that I dislike.)

frank


More information about the Squeak-dev mailing list