<div dir="ltr"><div class="gmail_extra"><br><br><div class="gmail_quote">2013/12/21 Frank Shearar <span dir="ltr"><<a href="mailto:frank.shearar@gmail.com" target="_blank">frank.shearar@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On 20 December 2013 21:16, Chris Muller <<a href="mailto:asqueaker@gmail.com">asqueaker@gmail.com</a>> wrote:<br>
> On Fri, Dec 20, 2013 at 3:01 PM, Frank Shearar <<a href="mailto:frank.shearar@gmail.com">frank.shearar@gmail.com</a>> wrote:<br>
>> On 20 December 2013 20:42, <<a href="mailto:commits@source.squeak.org">commits@source.squeak.org</a>> wrote:<br>
>>> Chris Muller uploaded a new version of Environments to project The Trunk:<br>
>>> <a href="http://source.squeak.org/trunk/Environments-cmm.37.mcz" target="_blank">http://source.squeak.org/trunk/Environments-cmm.37.mcz</a><br>
>>><br>
>>> ==================== Summary ====================<br>
>>><br>
>>> Name: Environments-cmm.37<br>
>>> Author: cmm<br>
>>> Time: 20 December 2013, 2:42:33.974 pm<br>
>>> UUID: c2e36521-ef29-4aa3-9425-84793a2d007c<br>
>>> Ancestors: Environments-nice.36<br>
>>><br>
>>> Minor cleanups while trying to learn Environments.<br>
>>><br>
>>> =============== Diff against Environments-nice.36 ===============<br>
>>><br>
>>> Item was changed:<br>
>>> ----- Method: AddPrefixNamePolicy class>>prefix: (in category 'as yet unclassified') -----<br>
>>> + prefix: aString<br>
>>> + ^ self new<br>
>>> + setPrefix: aString ;<br>
>>> + yourself!<br>
>>> - prefix: aString<br>
>>> - ^ self basicNew initializeWithPrefix: aString!<br>
>><br>
>> Why make the AddPrefixNamePolicy mutable? That seems like a bad idea.<br>
>> Unless there's some seriously major gain to be had, we should _not_<br>
>> add setters. The "basicNew initializeWithFoo:" idiom makes it clear<br>
>> that "AddPrefixNamePolicy new" will not result in a properly<br>
>> initialised object. It avoids giving people the impression that it's<br>
>> OK to monkey with the object's state. It's also the closest we can get<br>
>> to immutability, given that the only way we can initialise an object's<br>
>> state correctly is by sending it messages. (I realise the idiom's not<br>
>> in SBPP, but sometimes we can improve on the classics.)<br>
>><br>
>> I like how you renamed lots of parameters to indicate the kind of<br>
>> things they ought to contain ("aNamespace" -> "aDictionary"), but<br>
>> please, no setters.<br>
><br>
> Yeah, both things are simply documented best-practice conventions.<br>
> It's best not to call basicNew except for serialization frameworks or<br>
> otherwise with a good reason to, otherwise #initialize won't get<br>
> called from the one place it should be called from.<br>
<br>
</div></div>My point is that I don't think you should send #new at all, because<br>
that suggests that you could have an AddPrefixNamePolicy with no<br>
prefix.<br>
<div class="im"><br>
> So that requires changing the method name from #initializeWith...: to<br>
> simply #set...:, which is Kent Becks "Constructor Parameter Method" on<br>
> page 25 of his book.<br>
<br>
</div>I have a copy of SBPP (sadly not in the same country as me anymore),<br>
which is why I specifically suggested that sometimes we can improve on<br>
the classics :)<br>
<div class="im"><br>
> Now, I've heard the suggestion to name these methods as "myPrefix:<br>
> prefixString myNamespace: aDictionary", so that, should a developer<br>
> ever think he wants to use them, it will look and read awkwardly in<br>
> the calling code.<br>
><br>
> But that relies on a convention for "privatizing" that no other<br>
> private methods can afford.<br>
<br>
</div>As does that #new prefix: cascade.<br>
<div class="im"><br>
> Therefore, nothing besides the categories/protocols are either needed,<br>
> nor should be used, to indicate whether a method is ok to use from the<br>
> outside.<br>
><br>
> And, I confess, Beck says Constructor Parameter Methods should be<br>
> categorized as 'private' but I trumped him because<br>
> "initialize/release" is just as private (e.g., no one calls those from<br>
> the outside) plus.. that's what it is for! And, it co-locates it with<br>
> #initialize where it belongs and easy to see all initialization under<br>
> one protocol.<br>
><br>
> Whew!<br>
<br>
</div>I almost entirely agree with you. Except that I think that calling<br>
#basicNew and initialising within the #initializeWithFoo: methods is<br>
superior because<br>
* you don't need a setter (as in, you don't need a method that looks<br>
and smells and tastes exactly like a classic<br>
expose-my-state-for-mutation setter.<br>
* #initializeWithFoo: tells you the ways you can construct a valid<br>
instance, and if you want to construct an _invalid_ one you have to<br>
work at it - actively subvert the API.<br>
* #initializeWithFoo: tells you much louder exactly what it will do,<br>
where #setFoo: (a) looks like a Java-style mutator and (b) suggests<br>
that you can (partly!) _reinitialise_ an object. Worse, #setFoo: only<br>
partially initialises the object, where #initializeWithFoo: completely<br>
initialises an object.<br>
<br>
So in the end we're arguing about two conventions that do more or less<br>
the same thing. I don't see any _improvement_ in using #new, and in<br>
fact see several (admittedly minor) downsides. So what's the point<br>
then of "fixing" this?<br>
<br>
(As I already said, I think the parameter renaming's a great idea.<br>
It's just the #new setFoo: stuff that I dislike.)<br>
<span class="HOEnZb"><font color="#888888"><br>
frank<br>
<br>
</font></span></blockquote></div><br></div><div class="gmail_extra">Yep, I'm completely in line with Frank on this particular point.<br></div></div>