The Trunk: Environments-cmm.38.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Environments-cmm.38.mcz

commits-2
Chris Muller uploaded a new version of Environments to project The Trunk:
http://source.squeak.org/trunk/Environments-cmm.38.mcz

==================== Summary ====================

Name: Environments-cmm.38
Author: cmm
Time: 22 December 2013, 11:23:45.064 am
UUID: bcae5b17-4e55-4b16-be84-90fc1e283ae4
Ancestors: Environments-cmm.37

Use proper implementation of Constructor Parameter Method.

=============== Diff against Environments-cmm.37 ===============

Item was changed:
  ----- Method: EnvironmentInfo class>>name:organization:packages: (in category 'as yet unclassified') -----
  name: aString organization: aSystemOrganizer packages: aPackageOrganizer
+ ^ self new
+ setName: aString
- ^ self basicNew
- initializeWithName: aString
  organization: aSystemOrganizer
  packages: aPackageOrganizer!

Item was removed:
- ----- Method: EnvironmentInfo>>initializeWithName:organization:packages: (in category 'as yet unclassified') -----
- initializeWithName: aString organization: aSystemOrganizer packages: aPackageOrganizer
- self initialize.
- name := aString.
- organization := aSystemOrganizer.
- packages := aPackageOrganizer.
- !

Item was changed:
+ ----- Method: EnvironmentInfo>>name (in category 'access') -----
- ----- Method: EnvironmentInfo>>name (in category 'as yet unclassified') -----
  name
  ^ name!

Item was changed:
+ ----- Method: EnvironmentInfo>>organization (in category 'access') -----
- ----- Method: EnvironmentInfo>>organization (in category 'as yet unclassified') -----
  organization
  ^ organization!

Item was changed:
+ ----- Method: EnvironmentInfo>>packages (in category 'access') -----
- ----- Method: EnvironmentInfo>>packages (in category 'as yet unclassified') -----
  packages
  ^ packages!

Item was changed:
+ ----- Method: EnvironmentInfo>>printOn: (in category 'printing') -----
- ----- Method: EnvironmentInfo>>printOn: (in category 'as yet unclassified') -----
  printOn: aStream
  aStream nextPutAll: name.
  aStream nextPutAll: 'Info'!

Item was added:
+ ----- Method: EnvironmentInfo>>setName:organization:packages: (in category 'initialize-release') -----
+ setName: aString organization: aSystemOrganizer packages: aPackageOrganizer
+ name := aString.
+ organization := aSystemOrganizer.
+ packages := aPackageOrganizer!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Chris Muller-3
So here is another correct example of CPM when more than one variable
needs initialized.

Your home-brew version of CPM does the same thing, but it 1) violates
the rule to say things once and only once, 2) has #initialize getting
called from any of half-a-dozen places rather than the ONE place, 3)
doesn't factor the behavior of simply *constructing* a _well-formed
object_ from the behavior of fully initializing it (i.e.., building up
a large cache).

>> > 1) each class that declares instance variables has exactly one
>> > initialization method
>>
>> Well then you can't have more than one constructor type then.  Not all
>> objects can follow a path down to a single constructor on the
>> class-side that ends up calling #initializeWtih:...
>
> Why not? I've never found this to be an issue.
>
>>
>> In several cases,
>> you'll need #initializeWithThis: and #initializeWithThat:.  And, in
>> both cases, you want to call #initialize from the top of them.  That's
>> repeating yourself and unconventional.
>
> Right. That's why the rule is one initializer.

What is an "initializer?"  Is that an #initialize method or an
#initializeWith... method?  If the former, it makes no sense to say
it's a rule to only have one because that's all you COULD have.  If
the latter, you're contradicting yourself in the above paragraph.

>> > 2) each initialization method must leave the instance in a valid state
>>
>> Of course, as does any initialization method like CPM methods.
>
> Not so. If your class has ivars 'foo' and 'bar' and it needs both of them to
> have values for the object to be valid, then neither #setFoo: nor #setBar:
> leave the object in a valid state. You have to call both of them to get a
> fully-initialized object. But #initializeWithFoo:bar: leaves the object in a
> valid state.

The above makes it very clear you haven't read CPM.

That's why I've committed this change, as second example of CPM
showing it does the same thing, but without repeating itself and
without the other issues associated with the home-brew version.

>> > 3) each initialization method is named verbosely starting with
>> > 'initializeWith' and describes its parameters
>>
>> The ONLY reason you call it "initializeWithThis:that:other:thing:"
>> instead of "setThis:that:other:thing:" is because you want to repeat
>> yourself by calling #initalize at the top of each method.
>
> The reason to call it #initializeWithThis:that: is that it initializes the
> object with this and that. If you like consider this an instance of
> "intention-revealing selector".

But what if I ONLY want a minimally well-formed instance, (e.g.,
minimum number of inst-vars set to make a well-formed instance, but
not, say, 'cache' variables which are derived from the minimum vars --
CPM does the minimum, initializeWith:... implies to build the cache as
well).

>> > 6) there is one or more class-side constructors which send #basicNew and
>> > the
>> > initialization message
>>
>> Why basicNew instead of new?  Honestly, there's no good reason for
>> that and it forces you to repeat yourself in all of your
>> #initializeWith: methods.
>
> Because #new calls #initialize. When following this pattern, #initialize
> still gets called, but by a subclass initializer, not by the constructor. We
> want to avoid calling #initialize twice.

Exactly.  You've articulated one of the problems with your home-brew
CPM.  You're doing this unnecessary dance with calling #basicNew and
#initialize yourself when all you need to say is, simply, #new.
That's a definite regression of CPM.

On Sun, Dec 22, 2013 at 11:23 AM,  <[hidden email]> wrote:

> Chris Muller uploaded a new version of Environments to project The Trunk:
> http://source.squeak.org/trunk/Environments-cmm.38.mcz
>
> ==================== Summary ====================
>
> Name: Environments-cmm.38
> Author: cmm
> Time: 22 December 2013, 11:23:45.064 am
> UUID: bcae5b17-4e55-4b16-be84-90fc1e283ae4
> Ancestors: Environments-cmm.37
>
> Use proper implementation of Constructor Parameter Method.
>
> =============== Diff against Environments-cmm.37 ===============
>
> Item was changed:
>   ----- Method: EnvironmentInfo class>>name:organization:packages: (in category 'as yet unclassified') -----
>   name: aString organization: aSystemOrganizer packages: aPackageOrganizer
> +       ^ self new
> +               setName: aString
> -       ^ self basicNew
> -               initializeWithName: aString
>                 organization: aSystemOrganizer
>                 packages: aPackageOrganizer!
>
> Item was removed:
> - ----- Method: EnvironmentInfo>>initializeWithName:organization:packages: (in category 'as yet unclassified') -----
> - initializeWithName: aString organization: aSystemOrganizer packages: aPackageOrganizer
> -       self initialize.
> -       name := aString.
> -       organization := aSystemOrganizer.
> -       packages := aPackageOrganizer.
> -       !
>
> Item was changed:
> + ----- Method: EnvironmentInfo>>name (in category 'access') -----
> - ----- Method: EnvironmentInfo>>name (in category 'as yet unclassified') -----
>   name
>         ^ name!
>
> Item was changed:
> + ----- Method: EnvironmentInfo>>organization (in category 'access') -----
> - ----- Method: EnvironmentInfo>>organization (in category 'as yet unclassified') -----
>   organization
>         ^ organization!
>
> Item was changed:
> + ----- Method: EnvironmentInfo>>packages (in category 'access') -----
> - ----- Method: EnvironmentInfo>>packages (in category 'as yet unclassified') -----
>   packages
>         ^ packages!
>
> Item was changed:
> + ----- Method: EnvironmentInfo>>printOn: (in category 'printing') -----
> - ----- Method: EnvironmentInfo>>printOn: (in category 'as yet unclassified') -----
>   printOn: aStream
>         aStream nextPutAll: name.
>         aStream nextPutAll: 'Info'!
>
> Item was added:
> + ----- Method: EnvironmentInfo>>setName:organization:packages: (in category 'initialize-release') -----
> + setName: aString organization: aSystemOrganizer packages: aPackageOrganizer
> +       name := aString.
> +       organization := aSystemOrganizer.
> +       packages := aPackageOrganizer!
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Tobias Pape
I just want to point out that I really
dislike the

 ^ Foo new
        setBla: …

pattern.
I see you explain it and give good reason, but
I just want to express my discomfort.

Best
        -Tobias

On 22.12.2013, at 18:43, Chris Muller <[hidden email]> wrote:

> So here is another correct example of CPM when more than one variable
> needs initialized.
>
> Your home-brew version of CPM does the same thing, but it 1) violates
> the rule to say things once and only once, 2) has #initialize getting
> called from any of half-a-dozen places rather than the ONE place, 3)
> doesn't factor the behavior of simply *constructing* a _well-formed
> object_ from the behavior of fully initializing it (i.e.., building up
> a large cache).
>
>>>> 1) each class that declares instance variables has exactly one
>>>> initialization method
>>>
>>> Well then you can't have more than one constructor type then.  Not all
>>> objects can follow a path down to a single constructor on the
>>> class-side that ends up calling #initializeWtih:...
>>
>> Why not? I've never found this to be an issue.
>>
>>>
>>> In several cases,
>>> you'll need #initializeWithThis: and #initializeWithThat:.  And, in
>>> both cases, you want to call #initialize from the top of them.  That's
>>> repeating yourself and unconventional.
>>
>> Right. That's why the rule is one initializer.
>
> What is an "initializer?"  Is that an #initialize method or an
> #initializeWith... method?  If the former, it makes no sense to say
> it's a rule to only have one because that's all you COULD have.  If
> the latter, you're contradicting yourself in the above paragraph.
>
>>>> 2) each initialization method must leave the instance in a valid state
>>>
>>> Of course, as does any initialization method like CPM methods.
>>
>> Not so. If your class has ivars 'foo' and 'bar' and it needs both of them to
>> have values for the object to be valid, then neither #setFoo: nor #setBar:
>> leave the object in a valid state. You have to call both of them to get a
>> fully-initialized object. But #initializeWithFoo:bar: leaves the object in a
>> valid state.
>
> The above makes it very clear you haven't read CPM.
>
> That's why I've committed this change, as second example of CPM
> showing it does the same thing, but without repeating itself and
> without the other issues associated with the home-brew version.
>
>>>> 3) each initialization method is named verbosely starting with
>>>> 'initializeWith' and describes its parameters
>>>
>>> The ONLY reason you call it "initializeWithThis:that:other:thing:"
>>> instead of "setThis:that:other:thing:" is because you want to repeat
>>> yourself by calling #initalize at the top of each method.
>>
>> The reason to call it #initializeWithThis:that: is that it initializes the
>> object with this and that. If you like consider this an instance of
>> "intention-revealing selector".
>
> But what if I ONLY want a minimally well-formed instance, (e.g.,
> minimum number of inst-vars set to make a well-formed instance, but
> not, say, 'cache' variables which are derived from the minimum vars --
> CPM does the minimum, initializeWith:... implies to build the cache as
> well).
>
>>>> 6) there is one or more class-side constructors which send #basicNew and
>>>> the
>>>> initialization message
>>>
>>> Why basicNew instead of new?  Honestly, there's no good reason for
>>> that and it forces you to repeat yourself in all of your
>>> #initializeWith: methods.
>>
>> Because #new calls #initialize. When following this pattern, #initialize
>> still gets called, but by a subclass initializer, not by the constructor. We
>> want to avoid calling #initialize twice.
>
> Exactly.  You've articulated one of the problems with your home-brew
> CPM.  You're doing this unnecessary dance with calling #basicNew and
> #initialize yourself when all you need to say is, simply, #new.
> That's a definite regression of CPM.
>
> On Sun, Dec 22, 2013 at 11:23 AM,  <[hidden email]> wrote:
>> Chris Muller uploaded a new version of Environments to project The Trunk:
>> http://source.squeak.org/trunk/Environments-cmm.38.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Environments-cmm.38
>> Author: cmm
>> Time: 22 December 2013, 11:23:45.064 am
>> UUID: bcae5b17-4e55-4b16-be84-90fc1e283ae4
>> Ancestors: Environments-cmm.37
>>
>> Use proper implementation of Constructor Parameter Method.
>>
>> =============== Diff against Environments-cmm.37 ===============
>>
>> Item was changed:
>>  ----- Method: EnvironmentInfo class>>name:organization:packages: (in category 'as yet unclassified') -----
>>  name: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> +       ^ self new
>> +               setName: aString
>> -       ^ self basicNew
>> -               initializeWithName: aString
>>                organization: aSystemOrganizer
>>                packages: aPackageOrganizer!
>>
>> Item was removed:
>> - ----- Method: EnvironmentInfo>>initializeWithName:organization:packages: (in category 'as yet unclassified') -----
>> - initializeWithName: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> -       self initialize.
>> -       name := aString.
>> -       organization := aSystemOrganizer.
>> -       packages := aPackageOrganizer.
>> -       !
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>name (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>name (in category 'as yet unclassified') -----
>>  name
>>        ^ name!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>organization (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>organization (in category 'as yet unclassified') -----
>>  organization
>>        ^ organization!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>packages (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>packages (in category 'as yet unclassified') -----
>>  packages
>>        ^ packages!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>printOn: (in category 'printing') -----
>> - ----- Method: EnvironmentInfo>>printOn: (in category 'as yet unclassified') -----
>>  printOn: aStream
>>        aStream nextPutAll: name.
>>        aStream nextPutAll: 'Info'!
>>
>> Item was added:
>> + ----- Method: EnvironmentInfo>>setName:organization:packages: (in category 'initialize-release') -----
>> + setName: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> +       name := aString.
>> +       organization := aSystemOrganizer.
>> +       packages := aPackageOrganizer!
>>
>>
>



signature.asc (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Levente Uzonyi-2
In reply to this post by Chris Muller-3
On Sun, 22 Dec 2013, Chris Muller wrote:

> So here is another correct example of CPM when more than one variable
> needs initialized.
>
> Your home-brew version of CPM does the same thing, but it 1) violates
> the rule to say things once and only once, 2) has #initialize getting
> called from any of half-a-dozen places rather than the ONE place, 3)
> doesn't factor the behavior of simply *constructing* a _well-formed
> object_ from the behavior of fully initializing it (i.e.., building up
> a large cache).

If you follow this pattern, then #initialize will become a lie, because
it's not initializing anything. You do the actual initialization in the
#set* methods.
Also your third point applies to this CPM method (or what) too, because
#new will not create a _well-formed object_, the #set* methods will do
that. (Yes, there are cases when an object without being initialized
_with a parameter_ is not a well-formed object).

You're thinking more into #initialize than what it actually is. It's
nothing more than a hook, which is usually called when an object is
created, but there are no guarantees. I may override #new whenever I want,
or I might call #basicNew. I'm sure there are plenty of classes in the
current Trunk, which don't use #initialize at all.


Levente

P.S.: AFAIK #initialize is a "new concept", which was not part of ST-80.
P.P.S.: Just because someone (Kent Beck in this case) is doing something
differently than you, it doesn't mean it's better than what you do.
P.P.P.S.: Most Smalltalkers dislike the #set* methods for some reason.

>
>>>> 1) each class that declares instance variables has exactly one
>>>> initialization method
>>>
>>> Well then you can't have more than one constructor type then.  Not all
>>> objects can follow a path down to a single constructor on the
>>> class-side that ends up calling #initializeWtih:...
>>
>> Why not? I've never found this to be an issue.
>>
>>>
>>> In several cases,
>>> you'll need #initializeWithThis: and #initializeWithThat:.  And, in
>>> both cases, you want to call #initialize from the top of them.  That's
>>> repeating yourself and unconventional.
>>
>> Right. That's why the rule is one initializer.
>
> What is an "initializer?"  Is that an #initialize method or an
> #initializeWith... method?  If the former, it makes no sense to say
> it's a rule to only have one because that's all you COULD have.  If
> the latter, you're contradicting yourself in the above paragraph.
>
>>>> 2) each initialization method must leave the instance in a valid state
>>>
>>> Of course, as does any initialization method like CPM methods.
>>
>> Not so. If your class has ivars 'foo' and 'bar' and it needs both of them to
>> have values for the object to be valid, then neither #setFoo: nor #setBar:
>> leave the object in a valid state. You have to call both of them to get a
>> fully-initialized object. But #initializeWithFoo:bar: leaves the object in a
>> valid state.
>
> The above makes it very clear you haven't read CPM.
>
> That's why I've committed this change, as second example of CPM
> showing it does the same thing, but without repeating itself and
> without the other issues associated with the home-brew version.
>
>>>> 3) each initialization method is named verbosely starting with
>>>> 'initializeWith' and describes its parameters
>>>
>>> The ONLY reason you call it "initializeWithThis:that:other:thing:"
>>> instead of "setThis:that:other:thing:" is because you want to repeat
>>> yourself by calling #initalize at the top of each method.
>>
>> The reason to call it #initializeWithThis:that: is that it initializes the
>> object with this and that. If you like consider this an instance of
>> "intention-revealing selector".
>
> But what if I ONLY want a minimally well-formed instance, (e.g.,
> minimum number of inst-vars set to make a well-formed instance, but
> not, say, 'cache' variables which are derived from the minimum vars --
> CPM does the minimum, initializeWith:... implies to build the cache as
> well).
>
>>>> 6) there is one or more class-side constructors which send #basicNew and
>>>> the
>>>> initialization message
>>>
>>> Why basicNew instead of new?  Honestly, there's no good reason for
>>> that and it forces you to repeat yourself in all of your
>>> #initializeWith: methods.
>>
>> Because #new calls #initialize. When following this pattern, #initialize
>> still gets called, but by a subclass initializer, not by the constructor. We
>> want to avoid calling #initialize twice.
>
> Exactly.  You've articulated one of the problems with your home-brew
> CPM.  You're doing this unnecessary dance with calling #basicNew and
> #initialize yourself when all you need to say is, simply, #new.
> That's a definite regression of CPM.
>
> On Sun, Dec 22, 2013 at 11:23 AM,  <[hidden email]> wrote:
>> Chris Muller uploaded a new version of Environments to project The Trunk:
>> http://source.squeak.org/trunk/Environments-cmm.38.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Environments-cmm.38
>> Author: cmm
>> Time: 22 December 2013, 11:23:45.064 am
>> UUID: bcae5b17-4e55-4b16-be84-90fc1e283ae4
>> Ancestors: Environments-cmm.37
>>
>> Use proper implementation of Constructor Parameter Method.
>>
>> =============== Diff against Environments-cmm.37 ===============
>>
>> Item was changed:
>>   ----- Method: EnvironmentInfo class>>name:organization:packages: (in category 'as yet unclassified') -----
>>   name: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> +       ^ self new
>> +               setName: aString
>> -       ^ self basicNew
>> -               initializeWithName: aString
>>                 organization: aSystemOrganizer
>>                 packages: aPackageOrganizer!
>>
>> Item was removed:
>> - ----- Method: EnvironmentInfo>>initializeWithName:organization:packages: (in category 'as yet unclassified') -----
>> - initializeWithName: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> -       self initialize.
>> -       name := aString.
>> -       organization := aSystemOrganizer.
>> -       packages := aPackageOrganizer.
>> -       !
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>name (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>name (in category 'as yet unclassified') -----
>>   name
>>         ^ name!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>organization (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>organization (in category 'as yet unclassified') -----
>>   organization
>>         ^ organization!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>packages (in category 'access') -----
>> - ----- Method: EnvironmentInfo>>packages (in category 'as yet unclassified') -----
>>   packages
>>         ^ packages!
>>
>> Item was changed:
>> + ----- Method: EnvironmentInfo>>printOn: (in category 'printing') -----
>> - ----- Method: EnvironmentInfo>>printOn: (in category 'as yet unclassified') -----
>>   printOn: aStream
>>         aStream nextPutAll: name.
>>         aStream nextPutAll: 'Info'!
>>
>> Item was added:
>> + ----- Method: EnvironmentInfo>>setName:organization:packages: (in category 'initialize-release') -----
>> + setName: aString organization: aSystemOrganizer packages: aPackageOrganizer
>> +       name := aString.
>> +       organization := aSystemOrganizer.
>> +       packages := aPackageOrganizer!
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Nicolas Cellier

2013/12/23 Levente Uzonyi <[hidden email]>
On Sun, 22 Dec 2013, Chris Muller wrote:

So here is another correct example of CPM when more than one variable
needs initialized.

Your home-brew version of CPM does the same thing, but it 1) violates
the rule to say things once and only once, 2) has #initialize getting
called from any of half-a-dozen places rather than the ONE place, 3)
doesn't factor the behavior of simply *constructing* a _well-formed
object_ from the behavior of fully initializing it (i.e.., building up
a large cache).

If you follow this pattern, then #initialize will become a lie, because it's not initializing anything. You do the actual initialization in the #set* methods.
Also your third point applies to this CPM method (or what) too, because #new will not create a _well-formed object_, the #set* methods will do that. (Yes, there are cases when an object without being initialized _with a parameter_ is not a well-formed object).

You're thinking more into #initialize than what it actually is. It's nothing more than a hook, which is usually called when an object is created, but there are no guarantees. I may override #new whenever I want, or I might call #basicNew. I'm sure there are plenty of classes in the current Trunk, which don't use #initialize at all.


An example is Point. Point x: 4 y: 5 will send basicNew, not new.
Point does not define an initialize method. Well it could initialize x:=y:=0; but it doesn't.
Indeed, why would a point initialize x:=y:=0 to immediately change the values?
Initializing twice would just be a useless slowdown and is considered as an anti-pattern.
So Chris, please let us the freedom to not use set, and let instances be initialized by a single method, not a useless two phases (new->initialize)setThis:andThat:

Nicolas
 

Levente

P.S.: AFAIK #initialize is a "new concept", which was not part of ST-80.
P.P.S.: Just because someone (Kent Beck in this case) is doing something differently than you, it doesn't mean it's better than what you do.
P.P.P.S.: Most Smalltalkers dislike the #set* methods for some reason.


1) each class that declares instance variables has exactly one
initialization method

Well then you can't have more than one constructor type then.  Not all
objects can follow a path down to a single constructor on the
class-side that ends up calling #initializeWtih:...

Why not? I've never found this to be an issue.


In several cases,
you'll need #initializeWithThis: and #initializeWithThat:.  And, in
both cases, you want to call #initialize from the top of them.  That's
repeating yourself and unconventional.

Right. That's why the rule is one initializer.

What is an "initializer?"  Is that an #initialize method or an
#initializeWith... method?  If the former, it makes no sense to say
it's a rule to only have one because that's all you COULD have.  If
the latter, you're contradicting yourself in the above paragraph.

2) each initialization method must leave the instance in a valid state

Of course, as does any initialization method like CPM methods.

Not so. If your class has ivars 'foo' and 'bar' and it needs both of them to
have values for the object to be valid, then neither #setFoo: nor #setBar:
leave the object in a valid state. You have to call both of them to get a
fully-initialized object. But #initializeWithFoo:bar: leaves the object in a
valid state.

The above makes it very clear you haven't read CPM.

That's why I've committed this change, as second example of CPM
showing it does the same thing, but without repeating itself and
without the other issues associated with the home-brew version.

3) each initialization method is named verbosely starting with
'initializeWith' and describes its parameters

The ONLY reason you call it "initializeWithThis:that:other:thing:"
instead of "setThis:that:other:thing:" is because you want to repeat
yourself by calling #initalize at the top of each method.

The reason to call it #initializeWithThis:that: is that it initializes the
object with this and that. If you like consider this an instance of
"intention-revealing selector".

But what if I ONLY want a minimally well-formed instance, (e.g.,
minimum number of inst-vars set to make a well-formed instance, but
not, say, 'cache' variables which are derived from the minimum vars --
CPM does the minimum, initializeWith:... implies to build the cache as
well).

6) there is one or more class-side constructors which send #basicNew and
the
initialization message

Why basicNew instead of new?  Honestly, there's no good reason for
that and it forces you to repeat yourself in all of your
#initializeWith: methods.

Because #new calls #initialize. When following this pattern, #initialize
still gets called, but by a subclass initializer, not by the constructor. We
want to avoid calling #initialize twice.

Exactly.  You've articulated one of the problems with your home-brew
CPM.  You're doing this unnecessary dance with calling #basicNew and
#initialize yourself when all you need to say is, simply, #new.
That's a definite regression of CPM.

On Sun, Dec 22, 2013 at 11:23 AM,  <[hidden email]> wrote:
Chris Muller uploaded a new version of Environments to project The Trunk:
http://source.squeak.org/trunk/Environments-cmm.38.mcz

==================== Summary ====================

Name: Environments-cmm.38
Author: cmm
Time: 22 December 2013, 11:23:45.064 am
UUID: bcae5b17-4e55-4b16-be84-90fc1e283ae4
Ancestors: Environments-cmm.37

Use proper implementation of Constructor Parameter Method.

=============== Diff against Environments-cmm.37 ===============

Item was changed:
  ----- Method: EnvironmentInfo class>>name:organization:packages: (in category 'as yet unclassified') -----
  name: aString organization: aSystemOrganizer packages: aPackageOrganizer
+       ^ self new
+               setName: aString
-       ^ self basicNew
-               initializeWithName: aString
                organization: aSystemOrganizer
                packages: aPackageOrganizer!

Item was removed:
- ----- Method: EnvironmentInfo>>initializeWithName:organization:packages: (in category 'as yet unclassified') -----
- initializeWithName: aString organization: aSystemOrganizer packages: aPackageOrganizer
-       self initialize.
-       name := aString.
-       organization := aSystemOrganizer.
-       packages := aPackageOrganizer.
-       !

Item was changed:
+ ----- Method: EnvironmentInfo>>name (in category 'access') -----
- ----- Method: EnvironmentInfo>>name (in category 'as yet unclassified') -----
  name
        ^ name!

Item was changed:
+ ----- Method: EnvironmentInfo>>organization (in category 'access') -----
- ----- Method: EnvironmentInfo>>organization (in category 'as yet unclassified') -----
  organization
        ^ organization!

Item was changed:
+ ----- Method: EnvironmentInfo>>packages (in category 'access') -----
- ----- Method: EnvironmentInfo>>packages (in category 'as yet unclassified') -----
  packages
        ^ packages!

Item was changed:
+ ----- Method: EnvironmentInfo>>printOn: (in category 'printing') -----
- ----- Method: EnvironmentInfo>>printOn: (in category 'as yet unclassified') -----
  printOn: aStream
        aStream nextPutAll: name.
        aStream nextPutAll: 'Info'!

Item was added:
+ ----- Method: EnvironmentInfo>>setName:organization:packages: (in category 'initialize-release') -----
+ setName: aString organization: aSystemOrganizer packages: aPackageOrganizer
+       name := aString.
+       organization := aSystemOrganizer.
+       packages := aPackageOrganizer!








Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Colin Putney-3
In reply to this post by Chris Muller-3



On Sun, Dec 22, 2013 at 12:43 PM, Chris Muller <[hidden email]> wrote:
So here is another correct example of CPM when more than one variable
needs initialized.

Your home-brew version of CPM does the same thing,

If you need a name for it, let's call it the "Single Initializer" pattern. 
 
but it 1) violates
the rule to say things once and only once,

Huh? You keep saying this, but there's no duplication that I can see in the methods you removed. Could you point it out more explicitly?
 
2) has #initialize getting
called from any of half-a-dozen places rather than the ONE place

So? Sending a single message in more than one place is fine. It's what messages are for.
 
3)
doesn't factor the behavior of simply *constructing* a _well-formed
object_ from the behavior of fully initializing it (i.e.., building up
a large cache).

What are you talking about? None of the classes you've "cleaned up" has a cache. 

I think maybe some of your confusion comes from the fact that all these classes inherit from Object. That's a degenerate case, and the description of Single Initializer that I posted considers the more general case of a class hierarchy. I've attached a very simple example that might help clarify things. 

It might help to think of it this way: the fundamental principal is that every instance variable gets initialized exactly once. Since the variables in a given instance may be declared in different class in the hierarchy, each class provides an initialization method for the instance variables it declares. 

Colin



Example-Initialize.st (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Chris Muller-3
> On Sun, Dec 22, 2013 at 12:43 PM, Chris Muller <[hidden email]> wrote:
>>
>> So here is another correct example of CPM when more than one variable
>> needs initialized.
>>
>> Your home-brew version of CPM does the same thing,
>
>
> If you need a name for it, let's call it the "Single Initializer" pattern.

Why, it has the same number of "initializers" as CPM.  #initialize
(for default values) and #initializeWith:... for constructor
parameters.

>>
>> but it 1) violates
>>
>> the rule to say things once and only once,
>
> Huh? You keep saying this, but there's no duplication that I can see in the
> methods you removed. Could you point it out more explicitly?

You have "self initialize" in all of your #initializeWith:... because
you chose to call #basicNew instead of #new.  It's a throwback to
2002.

 >> 2) has #initialize getting
>> called from any of half-a-dozen places rather than the ONE place
>
> So? Sending a single message in more than one place is fine. It's what
> messages are for.

When you're duplicating the same code over and over again ("self
initialize") in all your subclasses, that should be a clue that you
should inherit that behavior from the superclass instead of repeating
yourself.

That's the standard followed by the rest of the image.

>> 3)
>> doesn't factor the behavior of simply *constructing* a _well-formed
>> object_ from the behavior of fully initializing it (i.e.., building up
>> a large cache).
>
> What are you talking about? None of the classes you've "cleaned up" has a
> cache.

The CPM pattern covers general case, not just your classes.

> I think maybe some of your confusion comes from the fact that all these
> classes inherit from Object. That's a degenerate case, and the description
> of Single Initializer that I posted considers the more general case of a
> class hierarchy. I've attached a very simple example that might help clarify
> things.

I'm not confused at all.  And, I think you and Frank implying I was
(while you clearly demonstrated YOU were) from the get-go of this
discussion was the genesis the "adversarial" nature of this
discussion.  You and I always seem to fall into that trap -- we should
find a way to avoid it so we can be more productive in the future.

> It might help to think of it this way: the fundamental principal is that
> every instance variable gets initialized exactly once. Since the variables
> in a given instance may be declared in different class in the hierarchy,
> each class provides an initialization method for the instance variables it
> declares.

Right, of course.  As opposed to what?  You're saying something above
which everyone already agrees on, and making it sound like CPM doesn't
do that or... what, I don't know.

Let's move on because I have more important, _design-level_ criticisms
of Environments to discuss.

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Colin Putney-3



On Mon, Dec 23, 2013 at 1:01 PM, Chris Muller <[hidden email]> wrote:
 
>> Your home-brew version of CPM does the same thing,
>
>
> If you need a name for it, let's call it the "Single Initializer" pattern.

Why, it has the same number of "initializers" as CPM.  #initialize
(for default values) and #initializeWith:... for constructor
parameters.

Because I find "your home-brew version of CPM" insulting. "Single Initializer" conveys the intent of the pattern pretty well, I think.
 
You have "self initialize" in all of your #initializeWith:... because
you chose to call #basicNew instead of #new.  It's a throwback to
2002.

Ah. Well, I'm not bothered by a single message send appearing in more than one place. 

Think of #new as a 0-parameter constructor. It sends #basicNew to get an instance, then sends #initialize to initialize its state.

Now consider the #prefix: constructor that you changed in AddPrefixNamePolicy. It's a 1-parameter constructor, which follows exactly the same pattern as #new, extended to take a parameter: it creates an instance with #basicNew, then sends #initializeWithPrefix: to set up the instance's state. EnvironmentInfo class>>name:organization:packages: also follows the pattern. It accepts 3 parameters, creates an instance with #basicNew and passes the parameters to the initializer. 

Because Smalltalk has keyword messages, we can't include general parameterized constructors in the base image, because they wouldn't have descriptive names. They'd be called something like #newWith: and #newWith:with:with:. It's better to let people define their own constructors. 

That gives me an idea, though. Instead of #prefix: and #name:organization:packages:, these constructors should be called #newWithPrefix: and #newWithName:organization:packages:. That would be clearer because it mirrors the structure of the initialization messages. 

See, this debate is useful after all. 

 >> 2) has #initialize getting
>> called from any of half-a-dozen places rather than the ONE place
>
> So? Sending a single message in more than one place is fine. It's what
> messages are for.

When you're duplicating the same code over and over again ("self
initialize") in all your subclasses, that should be a clue that you
should inherit that behavior from the superclass instead of repeating
yourself.

That's the standard followed by the rest of the image.

It looks like this is the crux of our disagreement. My response above applies here too.
 
>> 3)
>> doesn't factor the behavior of simply *constructing* a _well-formed
>> object_ from the behavior of fully initializing it (i.e.., building up
>> a large cache).
>
> What are you talking about? None of the classes you've "cleaned up" has a
> cache.

The CPM pattern covers general case, not just your classes.

Ok, then what makes you think that Single Initializer can't deal with caches well? If you have a case where you need to populate a cache separately from initializing the object to *have* a cache, well, then write your initializer that way. 
 
Let's move on because I have more important, _design-level_ criticisms
of Environments to discuss.

Sure, criticize away, I'm happy to discuss. But as long as you're committing your "clean ups" to the trunk, I don't want to just drop this. 

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Chris Muller-4
>> >> Your home-brew version of CPM does the same thing,
>> >
>> >
>> > If you need a name for it, let's call it the "Single Initializer"
>> > pattern.
>>
>> Why, it has the same number of "initializers" as CPM.  #initialize
>> (for default values) and #initializeWith:... for constructor
>> parameters.
>
> Because I find "your home-brew version of CPM" insulting. "Single

Insults are totally unproductive, I should have been more sensitive
with my words.  Apologies.

> Initializer" conveys the intent of the pattern pretty well, I think.

The reason I asked 'why' though was simply that 'Single', while maybe
the 95% case, is not the case 100% of the time, so I think it's a
misnomer for the name of your pattern.

You'd need more than one initializer whenever there's more than one
way to create an object.  Consider a NetworkLocation class -- which
might be created with an IP OR a hostname.  Whichever is specified,
the other could be derived but doesn't really need to be.  Therefore,
you don't need to cram it all through one single initializer that
parameterizes both.  You need two:  initializeWithHostname:port: and
initializeWithIpString:port:.

>> You have "self initialize" in all of your #initializeWith:... because
>> you chose to call #basicNew instead of #new.  It's a throwback to
>> 2002.
>
> Ah. Well, I'm not bothered by a single message send appearing in more than
> one place.
>
> Think of #new as a 0-parameter constructor. It sends #basicNew to get an
> instance, then sends #initialize to initialize its state.
>
> Now consider the #prefix: constructor that you changed in
> AddPrefixNamePolicy. It's a 1-parameter constructor, which follows exactly
> the same pattern as #new, extended to take a parameter: it creates an
> instance with #basicNew, then sends #initializeWithPrefix: to set up the
> instance's state. EnvironmentInfo class>>name:organization:packages: also
> follows the pattern. It accepts 3 parameters, creates an instance with
> #basicNew and passes the parameters to the initializer.
>
> Because Smalltalk has keyword messages, we can't include general
> parameterized constructors in the base image, because they wouldn't have
> descriptive names. They'd be called something like #newWith: and
> #newWith:with:with:. It's better to let people define their own
> constructors.
>
> That gives me an idea, though. Instead of #prefix: and
> #name:organization:packages:, these constructors should be called
> #newWithPrefix: and #newWithName:organization:packages:. That would be
> clearer because it mirrors the structure of the initialization messages.

So you think Point class>>x:y: should be #newWithX:y:?

That is just not how most Smalltalk code is written.  I believe there
is value to having unity with the rest of Smalltalk standards and
conventions.

Wow, who'd've ever thought *I* would be arguing for "aesthetics!"  :)

> See, this debate is useful after all.

It's a disaster.  :)

>>  >> 2) has #initialize getting
>> >> called from any of half-a-dozen places rather than the ONE place
>> >
>> > So? Sending a single message in more than one place is fine. It's what
>> > messages are for.
>>
>> When you're duplicating the same code over and over again ("self
>> initialize") in all your subclasses, that should be a clue that you
>> should inherit that behavior from the superclass instead of repeating
>> yourself.
>>
>> That's the standard followed by the rest of the image.
>
>
> It looks like this is the crux of our disagreement. My response above
> applies here too.
>
>>
>> >> 3)
>> >> doesn't factor the behavior of simply *constructing* a _well-formed
>> >> object_ from the behavior of fully initializing it (i.e.., building up
>> >> a large cache).
>> >
>> > What are you talking about? None of the classes you've "cleaned up" has
>> > a
>> > cache.
>>
>> The CPM pattern covers general case, not just your classes.
>
>
> Ok, then what makes you think that Single Initializer can't deal with caches
> well? If you have a case where you need to populate a cache separately from
> initializing the object to *have* a cache, well, then write your initializer
> that way.
>
>>
>> Let's move on because I have more important, _design-level_ criticisms
>> of Environments to discuss.
>
>
> Sure, criticize away, I'm happy to discuss. But as long as you're committing
> your "clean ups" to the trunk, I don't want to just drop this.

I was going to propose a compromise where I would give up my "set"
nomenclature which I strongly prefer and use your "initializeWith:" if
you would allow the "no-argument constructor" #new to be the one to
call #initialize, rather than from every #initializeWith: method.

But please feel free to do whatever you want.  Next time, I'll try
harder not to feel insulted myself, because I really do like y'all and
want us to be productive.

 - Chris

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Colin Putney-3



On Mon, Dec 23, 2013 at 3:34 PM, Chris Muller <[hidden email]> wrote:
 
The reason I asked 'why' though was simply that 'Single', while maybe
the 95% case, is not the case 100% of the time, so I think it's a
misnomer for the name of your pattern.

You'd need more than one initializer whenever there's more than one
way to create an object.  Consider a NetworkLocation class -- which
might be created with an IP OR a hostname.  Whichever is specified,
the other could be derived but doesn't really need to be.  Therefore,
you don't need to cram it all through one single initializer that
parameterizes both.  You need two:  initializeWithHostname:port: and
initializeWithIpString:port:.

No. Again, the whole point of the pattern is to have just one initializer per class. If you have more than one, you're defeating the purpose. For NetworkLocation, presumably you have just one variant stored internally. Let's say it's the IP address. In that case, you'd have one initializer, #initializeWithAddress:port: and two constructors, #newWithIpString:port: and #newWithHostname:port:. The constructors would be responsible for converting their parameters into the right form to pass to the initializer.

In the example I posted, Square uses this technique. 
 
So you think Point class>>x:y: should be #newWithX:y:?

No, because Point has been around forever, and changing it would create pointless *cough* breakage. 
 
That is just not how most Smalltalk code is written.  I believe there
is value to having unity with the rest of Smalltalk standards and
conventions.

Yeah, but Smalltalk doesn't really have a strong convention for constructor names. You see all sorts of conventions in the image and packages. But that was just an interesting idea that occurred to me. Ignore it if you don't like it. 

What's important about that passage was the bit about constructors. Your instinct to call the superclass constructor is spot on, but it should be done on the instance side. So instead of #newWithPrefix: calling #new, #initializeWithPrefix: calls #initialize.

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Tom Rushworth-2
On 13-12-23 13:13 , Colin Putney wrote:

> On Mon, Dec 23, 2013 at 3:34 PM, Chris Muller <[hidden email]> wrote:
>
>
>> The reason I asked 'why' though was simply that 'Single', while maybe
>> the 95% case, is not the case 100% of the time, so I think it's a
>> misnomer for the name of your pattern.
>>
>> You'd need more than one initializer whenever there's more than one
>> way to create an object.  Consider a NetworkLocation class -- which
>> might be created with an IP OR a hostname.  Whichever is specified,
>> the other could be derived but doesn't really need to be.  Therefore,
>> you don't need to cram it all through one single initializer that
>> parameterizes both.  You need two:  initializeWithHostname:port: and
>> initializeWithIpString:port:.
>
>
> No. Again, the whole point of the pattern is to have just one initializer
> per class.

I understood the pattern (coming from an Objective-C developer
viewpoint) not so much as having _one_, as having one _designated_
intiailizer that handles the most general case, dealing with all the
instance vars, any consistency issues, internal state etc.  It's fine to
have other initializers for special cases, but they must call the
designated one.

To use Point as a probably too simple example, the designated
initializer would be something like the existing:

   Point class>>x:y:

and then you might have another initializer something like

   Point class>>initAtIntersectionOf: line1 with: line2

but that method must call x:y:.  The pattern is that you have as many
initializers as makes sense, but they all end up calling the designated
one.  Some of the initializers may just supply default values, some may
do conversions from other objects, whatever makes sense for the problem
at hand.

Since Objective-C was starting fresh and didn't have an image to give
history so much weight :), they got to adopt a convention that all such
methods started with "init", and the methods corresponding to "new"
started with "alloc".  I think alloc and init make the roles clearer,
but that's the benefit of hindsight :).

 If you have more than one, you're defeating the purpose. For

> NetworkLocation, presumably you have just one variant stored internally.
> Let's say it's the IP address. In that case, you'd have one initializer,
> #initializeWithAddress:port: and two constructors, #newWithIpString:port:
> and #newWithHostname:port:. The constructors would be responsible for
> converting their parameters into the right form to pass to the initializer.
>
> In the example I posted, Square uses this technique.
>
>
>> So you think Point class>>x:y: should be #newWithX:y:?
>>
>
> No, because Point has been around forever, and changing it would create
> pointless *cough* breakage.
>
>
>> That is just not how most Smalltalk code is written.  I believe there
>> is value to having unity with the rest of Smalltalk standards and
>> conventions.
>>
>
> Yeah, but Smalltalk doesn't really have a strong convention for constructor
> names. You see all sorts of conventions in the image and packages. But that
> was just an interesting idea that occurred to me. Ignore it if you don't
> like it.
>
> What's important about that passage was the bit about constructors. Your
> instinct to call the superclass constructor is spot on, but it should be
> done on the instance side. So instead of #newWithPrefix: calling #new,
> #initializeWithPrefix: calls #initialize.
>
> Colin
>
>
>
>


--
Tom Rushworth

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Chris Muller-3
In reply to this post by Colin Putney-3
>> The reason I asked 'why' though was simply that 'Single', while maybe
>> the 95% case, is not the case 100% of the time, so I think it's a
>> misnomer for the name of your pattern.
>>
>> You'd need more than one initializer whenever there's more than one
>> way to create an object.  Consider a NetworkLocation class -- which
>> might be created with an IP OR a hostname.  Whichever is specified,
>> the other could be derived but doesn't really need to be.  Therefore,
>> you don't need to cram it all through one single initializer that
>> parameterizes both.  You need two:  initializeWithHostname:port: and
>> initializeWithIpString:port:.
>
> No. Again, the whole point of the pattern is to have just one initializer
> per class. If you have more than one, you're defeating the purpose. For
> NetworkLocation, presumably you have just one variant stored internally.
> Let's say it's the IP address.

See, I didn't want to get into this because your approach continues to
argue ways "it can be done" with Single Initializer and ignoring the
point I'm saying that sometimes there is an "either / or" scenario
that may not need, wish, or otherwise be-able-to, distill it down to
one single representation.

Maybe a requirement of the NeworkLocation is to be able to remember
exactly what the user specified to create it.  So it needs to keep the
original ipString or original hostnameString so that when they go back
to edit it they will see the familiar format that they entered and
prefer...

>  In that case, you'd have one initializer,
> #initializeWithAddress:port: and two constructors, #newWithIpString:port:
> and #newWithHostname:port:. The constructors would be responsible for
> converting their parameters into the right form to pass to the initializer.
>
> In the example I posted, Square uses this technique.

I have not see the example you refer to yet.

>> So you think Point class>>x:y: should be #newWithX:y:?
>
> No, because Point has been around forever, and changing it would create
> pointless *cough* breakage.

I meant, "should".  I know you aren't crazy enough to try to actually
do it.  I simply meant, IF you were back in time and you were writing
that constructor, you would prefer it as #newWithX:y:.

If so, I guess we'll just have to agree to disagree.

>> That is just not how most Smalltalk code is written.  I believe there
>> is value to having unity with the rest of Smalltalk standards and
>> conventions.
>
>
> Yeah, but Smalltalk doesn't really have a strong convention for constructor
> names. You see all sorts of conventions in the image and packages. But that
> was just an interesting idea that occurred to me. Ignore it if you don't
> like it.
>
> What's important about that passage was the bit about constructors. Your
> instinct to call the superclass constructor is spot on, but it should be
> done on the instance side. So instead of #newWithPrefix: calling #new,
> #initializeWithPrefix: calls #initialize.

Because "allocation of new instances of objects" is a _responsibility_
of the *class-side* and one of the things we vociferously argued about
years ago that this responsibility entails one thing, to #initialize
the object.  Your pattern takes that responsibility and, _sometimes_,
but not always, expects the end-users application to do it!  That's
quite a jump, and an inconsistent handling of the responsibility to
#initialize.

We'll just have to agree to disagree.

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Nicolas Cellier

2013/12/23 Chris Muller <[hidden email]>
>> The reason I asked 'why' though was simply that 'Single', while maybe
>> the 95% case, is not the case 100% of the time, so I think it's a
>> misnomer for the name of your pattern.
>>
>> You'd need more than one initializer whenever there's more than one
>> way to create an object.  Consider a NetworkLocation class -- which
>> might be created with an IP OR a hostname.  Whichever is specified,
>> the other could be derived but doesn't really need to be.  Therefore,
>> you don't need to cram it all through one single initializer that
>> parameterizes both.  You need two:  initializeWithHostname:port: and
>> initializeWithIpString:port:.
>
> No. Again, the whole point of the pattern is to have just one initializer
> per class. If you have more than one, you're defeating the purpose. For
> NetworkLocation, presumably you have just one variant stored internally.
> Let's say it's the IP address.

See, I didn't want to get into this because your approach continues to
argue ways "it can be done" with Single Initializer and ignoring the
point I'm saying that sometimes there is an "either / or" scenario
that may not need, wish, or otherwise be-able-to, distill it down to
one single representation.

Maybe a requirement of the NeworkLocation is to be able to remember
exactly what the user specified to create it.  So it needs to keep the
original ipString or original hostnameString so that when they go back
to edit it they will see the familiar format that they entered and
prefer...

>  In that case, you'd have one initializer,
> #initializeWithAddress:port: and two constructors, #newWithIpString:port:
> and #newWithHostname:port:. The constructors would be responsible for
> converting their parameters into the right form to pass to the initializer.
>
> In the example I posted, Square uses this technique.

I have not see the example you refer to yet.

>> So you think Point class>>x:y: should be #newWithX:y:?
>
> No, because Point has been around forever, and changing it would create
> pointless *cough* breakage.

I meant, "should".  I know you aren't crazy enough to try to actually
do it.  I simply meant, IF you were back in time and you were writing
that constructor, you would prefer it as #newWithX:y:.

If so, I guess we'll just have to agree to disagree.

>> That is just not how most Smalltalk code is written.  I believe there
>> is value to having unity with the rest of Smalltalk standards and
>> conventions.
>
>
> Yeah, but Smalltalk doesn't really have a strong convention for constructor
> names. You see all sorts of conventions in the image and packages. But that
> was just an interesting idea that occurred to me. Ignore it if you don't
> like it.
>
> What's important about that passage was the bit about constructors. Your
> instinct to call the superclass constructor is spot on, but it should be
> done on the instance side. So instead of #newWithPrefix: calling #new,
> #initializeWithPrefix: calls #initialize.

Because "allocation of new instances of objects" is a _responsibility_
of the *class-side* and one of the things we vociferously argued about
years ago that this responsibility entails one thing, to #initialize
the object.  Your pattern takes that responsibility and, _sometimes_,
but not always, expects the end-users application to do it!  That's
quite a jump, and an inconsistent handling of the responsibility to
#initialize.

We'll just have to agree to disagree.


No there are already different ways to initialize.
For example, take the collections, some of them #initialize: not #initialize in order to pass a size (or capacity) to the initializer.
So the pattern used by Colin is pretty common in Squeak.
I invoked the case of Point more for the fact it sends basicNew instead of new, than for the names.

There are indeed two different things on the table
1) to have a single initializer
2) the names chosen for initialization

Chris, the name count, you can't rename initialize -> set and declare this is not a setter, it's quite surrealist and make me think of http://en.wikipedia.org/wiki/The_Treachery_of_Images ;)

I suggest we concentrate on 1) - the single initializer pattern.


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Chris Muller-3
> No there are already different ways to initialize.
> For example, take the collections, some of them #initialize: not #initialize
> in order to pass a size (or capacity) to the initializer.
> So the pattern used by Colin is pretty common in Squeak.
> I invoked the case of Point more for the fact it sends basicNew instead of
> new, than for the names.

The only reason Point sends basicNew instead of new is for
performance.  Morphic applications create millions of Point instances
and so an exception is made to avoid the extra send, nothing more.

> There are indeed two different things on the table
> 1) to have a single initializer
> 2) the names chosen for initialization
>
> Chris, the name count, you can't rename initialize -> set and declare this
> is not a setter, it's quite surrealist and make me think of

You think the "set" prefix makes it a setter?  Even though no
Smalltalker prefixes their setters with "set" and also it sets
multiple variables and also is categorized under "initialize-release"
instead of "accessing"?

That is three differences from what we think of as a "setter".  But
you say something that is different in 3 ways is "surreal" to call it
something else..   You need to refine your pattern language!   :)

> http://en.wikipedia.org/wiki/The_Treachery_of_Images ;)
>
> I suggest we concentrate on 1) - the single initializer pattern.

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Nicolas Cellier
2013/12/24 Chris Muller <[hidden email]>
> No there are already different ways to initialize.
> For example, take the collections, some of them #initialize: not #initialize
> in order to pass a size (or capacity) to the initializer.
> So the pattern used by Colin is pretty common in Squeak.
> I invoked the case of Point more for the fact it sends basicNew instead of
> new, than for the names.

The only reason Point sends basicNew instead of new is for
performance.  Morphic applications create millions of Point instances
and so an exception is made to avoid the extra send, nothing more.


That's more or less the same of what I earlier said: you do not want instance variables to be initialized twice.
It's considered an anti-pattern for two reasons
1) useless waste if CPU cycles
2) useless trouble for who wants to understand how the class works (think of debugging)
 
> There are indeed two different things on the table
> 1) to have a single initializer
> 2) the names chosen for initialization
>
> Chris, the name count, you can't rename initialize -> set and declare this
> is not a setter, it's quite surrealist and make me think of

You think the "set" prefix makes it a setter?  Even though no
Smalltalker prefixes their setters with "set" and also it sets
multiple variables and also is categorized under "initialize-release"
instead of "accessing"?

That is three differences from what we think of as a "setter".  But
you say something that is different in 3 ways is "surreal" to call it
something else..   You need to refine your pattern language!   :)


Chris, the 3 arguments setter was added afterward.
The 1st method you modified and that made all of us react was setPrefix:
If it does not quake enough like a setter, then what does? Be fair.

I agree that a Smalltalk-centric setter would be named prefix: in our tradition.
But if you look a bit wider around us, setters are just named set (java C++ python whatever...).
So initializePrefix: is a clear disambiguation now that Smalltalk is not alone in its world.
And IMO, classification is a hint, but is not enough.

 
> http://en.wikipedia.org/wiki/The_Treachery_of_Images ;)
>
> I suggest we concentrate on 1) - the single initializer pattern.

But let's concentrate on this one as I suggested :)


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Colin Putney-3
In reply to this post by Chris Muller-3



On Mon, Dec 23, 2013 at 5:28 PM, Chris Muller <[hidden email]> wrote:
 
See, I didn't want to get into this because your approach continues to
argue ways "it can be done" with Single Initializer and ignoring the
point I'm saying that sometimes there is an "either / or" scenario
that may not need, wish, or otherwise be-able-to, distill it down to
one single representation.

Maybe a requirement of the NeworkLocation is to be able to remember
exactly what the user specified to create it.  So it needs to keep the
original ipString or original hostnameString so that when they go back
to edit it they will see the familiar format that they entered and
prefer...

An instance of a given class will always have the same set of instance variables, right? And for that set of instance variables, there's some combination of values that a valid instance could have. So write a method that sets the variables to those values. Maybe some of the parameters will be nil. That's fine. The only requirement is that it leave the object in a valid state. I can't see how there's any case where it's not possible to do this. 

On the other hand, sure, there may be cases where it's not convenient. In those cases, don't use Single Initializer. It's a pattern, not a commandment. 
  
I have not see the example you refer to yet.

Looks like the list-serv scrubbed it. Download it from here:

 
>> So you think Point class>>x:y: should be #newWithX:y:?
>
> No, because Point has been around forever, and changing it would create
> pointless *cough* breakage.

I meant, "should".  I know you aren't crazy enough to try to actually
do it.  I simply meant, IF you were back in time and you were writing
that constructor, you would prefer it as #newWithX:y:.

In a brand new Smalltalk with no legacy code, sure. It's a bit awkward, but given that we usually create points with x@y, it's fine. 

Because "allocation of new instances of objects" is a _responsibility_
of the *class-side* and one of the things we vociferously argued about
years ago that this responsibility entails one thing, to #initialize
the object.  Your pattern takes that responsibility and, _sometimes_,
but not always, expects the end-users application to do it!  That's
quite a jump, and an inconsistent handling of the responsibility to
#initialize.

Single Initializer always expects the class that declares instance variables to provide a method for initializing them. Object provides an empty #initialize method, because it has no instance variables. I don't see where the library/application distinction comes into it, nor any inconsistency. Could you elaborate?

Hmm, are you saying that #initialize should set default values, which would then be replaced by values passed to a Constructor Parameter Method? Using Point as an example:

Point>>initialize
    x := 0.
    y := 0.

Point>>setX: xInteger y: yInteger
   x := xInteger.
   y := yInteger.

Integer>>@anInteger
   ^ Point new; setX: self y: anInteger


That's reasonable, in that it never leaves the instance in an invalid state, but I don't like the repetition. If the creator of the point knows the values it wants, why not just set them once and be done with it?

We'll just have to agree to disagree.

Sure, I'm happy to do so.  I'm not trying to convince you to adopt my taste, just to stop rewriting my code to conform to yours.

Colin



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Colin Putney-3
In reply to this post by Tom Rushworth-2



On Mon, Dec 23, 2013 at 5:13 PM, Tom Rushworth <[hidden email]> wrote:
On 13-12-23 13:13 , Colin Putney wrote:
> On Mon, Dec 23, 2013 at 3:34 PM, Chris Muller <[hidden email]> wrote:
>
>
>> The reason I asked 'why' though was simply that 'Single', while maybe
>> the 95% case, is not the case 100% of the time, so I think it's a
>> misnomer for the name of your pattern.
>>
>> You'd need more than one initializer whenever there's more than one
>> way to create an object.  Consider a NetworkLocation class -- which
>> might be created with an IP OR a hostname.  Whichever is specified,
>> the other could be derived but doesn't really need to be.  Therefore,
>> you don't need to cram it all through one single initializer that
>> parameterizes both.  You need two:  initializeWithHostname:port: and
>> initializeWithIpString:port:.
>
>
> No. Again, the whole point of the pattern is to have just one initializer
> per class.

I understood the pattern (coming from an Objective-C developer
viewpoint) not so much as having _one_, as having one _designated_
intiailizer that handles the most general case, dealing with all the
instance vars, any consistency issues, internal state etc.  It's fine to
have other initializers for special cases, but they must call the
designated one.

To use Point as a probably too simple example, the designated
initializer would be something like the existing:

   Point class>>x:y:

and then you might have another initializer something like

   Point class>>initAtIntersectionOf: line1 with: line2

but that method must call x:y:.  The pattern is that you have as many
initializers as makes sense, but they all end up calling the designated
one.  Some of the initializers may just supply default values, some may
do conversions from other objects, whatever makes sense for the problem
at hand.

Since Objective-C was starting fresh and didn't have an image to give
history so much weight :), they got to adopt a convention that all such
methods started with "init", and the methods corresponding to "new"
started with "alloc".  I think alloc and init make the roles clearer,
but that's the benefit of hindsight :).

Hi Tom,

That's a good point. I've perhaps been overly emphatic about having just one initializer; perhaps the pattern ought to be called "Designated Initializer" instead. I think part of this is that the common Objective-C idiom of "[[SomeClass alloc] init]" isn't used in Smalltalk—we always extract that into a class-side constructor. Given that, it makes sense to do the necessary transformations in the constructor, and then have it call the designated initializer with parameters of the correct form. But sure, having #initializeAtIntersectionOf:with: (as an instance method!) call through to #initializeWithX:y: wouldn't bother me at all. 

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cmm.38.mcz

Nicolas Cellier
In reply to this post by Nicolas Cellier

2013/12/24 Nicolas Cellier <[hidden email]>
2013/12/24 Chris Muller <[hidden email]>
> No there are already different ways to initialize.
> For example, take the collections, some of them #initialize: not #initialize
> in order to pass a size (or capacity) to the initializer.
> So the pattern used by Colin is pretty common in Squeak.
> I invoked the case of Point more for the fact it sends basicNew instead of
> new, than for the names.

The only reason Point sends basicNew instead of new is for
performance.  Morphic applications create millions of Point instances
and so an exception is made to avoid the extra send, nothing more.


That's more or less the same of what I earlier said: you do not want instance variables to be initialized twice.
It's considered an anti-pattern for two reasons
1) useless waste if CPU cycles
2) useless trouble for who wants to understand how the class works (think of debugging)

Ah, I remember where I first saw this anti pattern now:
Recipe of "l'andouillon" in "L'écume des jours" from Boris Vian:
Graissez un moule et rangez-le pour qu'il ne rouille pas.
 
 
> There are indeed two different things on the table
> 1) to have a single initializer
> 2) the names chosen for initialization
>
> Chris, the name count, you can't rename initialize -> set and declare this
> is not a setter, it's quite surrealist and make me think of

You think the "set" prefix makes it a setter?  Even though no
Smalltalker prefixes their setters with "set" and also it sets
multiple variables and also is categorized under "initialize-release"
instead of "accessing"?

That is three differences from what we think of as a "setter".  But
you say something that is different in 3 ways is "surreal" to call it
something else..   You need to refine your pattern language!   :)


Chris, the 3 arguments setter was added afterward.
The 1st method you modified and that made all of us react was setPrefix:
If it does not quake enough like a setter, then what does? Be fair.

I agree that a Smalltalk-centric setter would be named prefix: in our tradition.
But if you look a bit wider around us, setters are just named set (java C++ python whatever...).
So initializePrefix: is a clear disambiguation now that Smalltalk is not alone in its world.
And IMO, classification is a hint, but is not enough.

 
> http://en.wikipedia.org/wiki/The_Treachery_of_Images ;)
>
> I suggest we concentrate on 1) - the single initializer pattern.

But let's concentrate on this one as I suggested :)