How can I make this more OOP

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

How can I make this more OOP

Pharo Smalltalk Users mailing list
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof

Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

N. Bouraqadi
Hi Roelof,

Polymorphism is the answer.

Object >> flattenInto: result
result nextPut: anObject

UndefinedObject >> flattenInto: result
^self

Collection >> flattenInto: result
self do: [ :item | item flattenInto: result ]

flatten: anObject
    ^ (OrderedCollection streamContents: [ :stream | anObject flattenInto: stream ]) asArray

Noury

On 13 Sep 2020, at 11:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:

Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof


Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Richard O'Keefe
In reply to this post by Pharo Smalltalk Users mailing list
"OOP is not asking an object what it is"?  You've lost me.
I'm reminded of a joke exam question that goes something
like this:
   Some things have ping nature and other things have pong nature.
   Discuss, with examples.

Whatever else it is, OOP is a means to an end, not an end in itself.
It's not a religion.  Allah will not cast you into the Fire for using
something which is not ritually pure.

The whole point of the Design Patterns movement was not to present
canned solutions but to describe common *situations* characterised
by (metaphorical) *forces* pushing you in incompatible directions.

Question 1: Why do you even have a stream there?
flattenArray: aCollection
    |buffer|
    buffer := OrderedCollection new: aCollection size.
    self flattenArray: aCollection into: buffer.
    ^buffer asArray

flattenArray: anObject into: buffer
    (anObject respondsTo: #do:)
       ifTrue:  [anObject do: [:each | self flattenArray: each into: buffer]
       ifFalse: [anObject ifNotNil: [buffer addLast: anObject].

(CAUTION: untested code.)

Question 2: is there any point in *trying* to make this more ping and less pong?
I have to say that in 40 years of programming, I have *never* wanted to flatten
a completely arbitrary structure.  When I have wanted to flatten something, it
has always been an instance of the Composite design pattern, so that a specific
and quite small set of classes has been involved.

I am well aware that some Smalltalk systems have some sort of 'flatten'
lying around like a rake in the grass, but I have found no two which agree
on the specification.

Question 3: Let's consider an if-less alternative.

Stream
  flattenInto: buffer
    self do: [:each| each flattenInto: buffer].

Collection
  flattenInto: buffer
    self do:[:each | each flattenInto: buffer].

Object
  flattenInto: buffer
    buffer addLast: self.

UndefinedObject
  flattenInto: buffer
    "Do nothing."

This is Smalltalk, where we *can* add methods to system classes like these.
When the payoff is worth it, I have no qualms whatever in doing so.
But putting methods of little or no utility into the interface of EVERY
object just feels so wrong.

Here we have opposing forces:
 - The first approach adds two methods to your worker class,
   and no methods to any system class.  Instead, it uses
   "does this object know how to iterate over its elements?"
   and "is this object nil?".  This does minimal damage to
   system structure at the price of a little ritual impurity.

 - The second approach adds a method to four system classes,
   enlarging the interface of every object in the system,
   creating a level of coupling we'd be better without, and
   making it harder for someone reading your code to figure
   out what's going on.

In the context of a Composite, with a limited number of
application classes involved, the second approach is better;
the method is/methods are not defined anywhere they should
not be.

In the context of *this* problem, the first approach is
better.  MUCH better.  Why?  Because *encapsulation* is much
more important to OOP than absence-of-IFs.

On Sun, 13 Sep 2020 at 21:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof

Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Pharo Smalltalk Users mailing list

Thanks Richard for this explanation.
and I like also your idea of using #respondTo instead of asking a object if its a collection or a item.

Roelof


Op 14-9-2020 om 14:28 schreef Richard O'Keefe:
"OOP is not asking an object what it is"?  You've lost me.
I'm reminded of a joke exam question that goes something
like this:
   Some things have ping nature and other things have pong nature.
   Discuss, with examples.

Whatever else it is, OOP is a means to an end, not an end in itself.
It's not a religion.  Allah will not cast you into the Fire for using
something which is not ritually pure.

The whole point of the Design Patterns movement was not to present
canned solutions but to describe common *situations* characterised
by (metaphorical) *forces* pushing you in incompatible directions.

Question 1: Why do you even have a stream there?
flattenArray: aCollection
    |buffer|
    buffer := OrderedCollection new: aCollection size.
    self flattenArray: aCollection into: buffer.
    ^buffer asArray

flattenArray: anObject into: buffer
    (anObject respondsTo: #do:)
       ifTrue:  [anObject do: [:each | self flattenArray: each into: buffer]
       ifFalse: [anObject ifNotNil: [buffer addLast: anObject].

(CAUTION: untested code.)

Question 2: is there any point in *trying* to make this more ping and less pong?
I have to say that in 40 years of programming, I have *never* wanted to flatten
a completely arbitrary structure.  When I have wanted to flatten something, it
has always been an instance of the Composite design pattern, so that a specific
and quite small set of classes has been involved.

I am well aware that some Smalltalk systems have some sort of 'flatten'
lying around like a rake in the grass, but I have found no two which agree
on the specification.

Question 3: Let's consider an if-less alternative.

Stream
  flattenInto: buffer
    self do: [:each| each flattenInto: buffer].

Collection
  flattenInto: buffer
    self do:[:each | each flattenInto: buffer].

Object
  flattenInto: buffer
    buffer addLast: self.

UndefinedObject
  flattenInto: buffer
    "Do nothing."

This is Smalltalk, where we *can* add methods to system classes like these.
When the payoff is worth it, I have no qualms whatever in doing so.
But putting methods of little or no utility into the interface of EVERY
object just feels so wrong.

Here we have opposing forces:
 - The first approach adds two methods to your worker class,
   and no methods to any system class.  Instead, it uses
   "does this object know how to iterate over its elements?"
   and "is this object nil?".  This does minimal damage to
   system structure at the price of a little ritual impurity.

 - The second approach adds a method to four system classes,
   enlarging the interface of every object in the system,
   creating a level of coupling we'd be better without, and
   making it harder for someone reading your code to figure
   out what's going on.

In the context of a Composite, with a limited number of
application classes involved, the second approach is better;
the method is/methods are not defined anywhere they should
not be.

In the context of *this* problem, the first approach is
better.  MUCH better.  Why?  Because *encapsulation* is much
more important to OOP than absence-of-IFs.

On Sun, 13 Sep 2020 at 21:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof


Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Sean P. DeNigris
Administrator
In reply to this post by Richard O'Keefe
Richard O'Keefe wrote
> Whatever else it is, OOP is a means to an end, not an end in itself.
> It's not a religion.

Richard makes an important point here.

As I mentioned on Discord (slightly edited to fit this thread), it’s a
judgment call with trade-offs, but for the purposes of an OOP exercise, I
would say absolutely put the methods in object and wherever else you may
need them - if only to get practice with that particular idiom. I needed to
do a lot of that sort of thing - probably way *too* much, just to break my
procedural thinking habits. Once I got it "in my bones" I became more
pragmatic. While in theory, this may always be the “most“ OOP way, in
practice it can be confusing due to the limitations of our system
organization and the fact that people will have to scroll through more
methods on kernel objects (not to mention the extra dependencies Richard
noted). That is the dogma, and I think is a fine answer for an OOP exercise,
but IMHO in practice there are justifiable exceptions. The two major axes
are understandability and adaptability. If in your domain there are only two
choices and there can not logically be more, it may be easier to understand
one method with a conditional that documents that domain fact, than it would
be to dive down a polymorphism rabbit hole spread across the system. Of
course, tooling is a big factor in understandability. In GT it may be
trivial because you can open implementors in a plane right next to the code
browser. Different toolsets themselves mean different trade offs. There are
no one size fits all answers for all time. There are principles that have to
be negotiated with your current environment, use case, experience level,
team size...



-----
Cheers,
Sean
--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Richard O'Keefe
I take Sean's point.  However, while I agree that practising using
polymorphism instead of 'if' is a good idea, I don't think that
*this* exercise is a good one for getting such practice.
Roelof is going through the Pharo track of Exercism.
This particular one is called 'flatten-array'.
The one concession to "hey, this is Pharo!" in the description
of the exercise is a veiled warning against using Pharo's
#flattened/#flattenOn:, which
(a) does not add anything to Object or UndefinedObject
    (just Collection>>flattened and Collection>>flattenOn:)
(b) does not make a special case of nil
(c) DOES make a special case of strings, treating them as atomic.
Possibly the most challenging part for me in this exercise was
trying to figure out "what does <list> mean for Smalltalk here?"
In Pharo's existing #flattened, why does a String count as atomic
but a Semphore is just another collection?

This is a Lisp programming exercise from the early 1960s.
It had unpleasant ambiguities back then and it has not got
better since.

If this were rewritten as
  "Make a Composite implementation of binary search trees
   with a class for empty trees, a class for non-empty ones,
   and perhaps a class for singleton ones.
   Implement #do:, traversing the elements in increasing order.
   Use this to implement #asOrderedCollection."
then that *would* be a good exercise for using polymorphism
instead of conditionals.

BST ()
  asOrderedCollection
    ^OrderedCollection new in: [:coll |
       self do: [:each | coll addLast: each].
       coll]
  EmptyBST ()
    do: aBlock
      "Nothing to do."
  NonemptyBST (less key greater)
    do: aBlock
      less do: aBlock.
      aBlock value: key.
      greater do: aBlock.




On Tue, 15 Sep 2020 at 18:33, Sean P. DeNigris <[hidden email]> wrote:
Richard O'Keefe wrote
> Whatever else it is, OOP is a means to an end, not an end in itself.
> It's not a religion.

Richard makes an important point here.

As I mentioned on Discord (slightly edited to fit this thread), it’s a
judgment call with trade-offs, but for the purposes of an OOP exercise, I
would say absolutely put the methods in object and wherever else you may
need them - if only to get practice with that particular idiom. I needed to
do a lot of that sort of thing - probably way *too* much, just to break my
procedural thinking habits. Once I got it "in my bones" I became more
pragmatic. While in theory, this may always be the “most“ OOP way, in
practice it can be confusing due to the limitations of our system
organization and the fact that people will have to scroll through more
methods on kernel objects (not to mention the extra dependencies Richard
noted). That is the dogma, and I think is a fine answer for an OOP exercise,
but IMHO in practice there are justifiable exceptions. The two major axes
are understandability and adaptability. If in your domain there are only two
choices and there can not logically be more, it may be easier to understand
one method with a conditional that documents that domain fact, than it would
be to dive down a polymorphism rabbit hole spread across the system. Of
course, tooling is a big factor in understandability. In GT it may be
trivial because you can open implementors in a plane right next to the code
browser. Different toolsets themselves mean different trade offs. There are
no one size fits all answers for all time. There are principles that have to
be negotiated with your current environment, use case, experience level,
team size...



-----
Cheers,
Sean
--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Stéphane Ducasse
In reply to this post by Pharo Smalltalk Users mailing list


On 14 Sep 2020, at 16:02, Roelof Wobben via Pharo-users <[hidden email]> wrote:


Thanks Richard for this explanation.
and I like also your idea of using #respondTo instead of asking a object if its a collection or a item.

refrain from using respondTo:
I make code difficult to evolve and can introduce vicious bugs.

S. 
Roelof


Op 14-9-2020 om 14:28 schreef Richard O'Keefe:
"OOP is not asking an object what it is"?  You've lost me.
I'm reminded of a joke exam question that goes something
like this:
   Some things have ping nature and other things have pong nature.
   Discuss, with examples.

Whatever else it is, OOP is a means to an end, not an end in itself.
It's not a religion.  Allah will not cast you into the Fire for using
something which is not ritually pure.

The whole point of the Design Patterns movement was not to present
canned solutions but to describe common *situations* characterised
by (metaphorical) *forces* pushing you in incompatible directions.

Question 1: Why do you even have a stream there?
flattenArray: aCollection
    |buffer|
    buffer := OrderedCollection new: aCollection size.
    self flattenArray: aCollection into: buffer.
    ^buffer asArray

flattenArray: anObject into: buffer
    (anObject respondsTo: #do:)
       ifTrue:  [anObject do: [:each | self flattenArray: each into: buffer]
       ifFalse: [anObject ifNotNil: [buffer addLast: anObject].

(CAUTION: untested code.)

Question 2: is there any point in *trying* to make this more ping and less pong?
I have to say that in 40 years of programming, I have *never* wanted to flatten
a completely arbitrary structure.  When I have wanted to flatten something, it
has always been an instance of the Composite design pattern, so that a specific
and quite small set of classes has been involved.

I am well aware that some Smalltalk systems have some sort of 'flatten'
lying around like a rake in the grass, but I have found no two which agree
on the specification.

Question 3: Let's consider an if-less alternative.

Stream
  flattenInto: buffer
    self do: [:each| each flattenInto: buffer].

Collection
  flattenInto: buffer
    self do:[:each | each flattenInto: buffer].

Object
  flattenInto: buffer
    buffer addLast: self.

UndefinedObject
  flattenInto: buffer
    "Do nothing."

This is Smalltalk, where we *can* add methods to system classes like these.
When the payoff is worth it, I have no qualms whatever in doing so.
But putting methods of little or no utility into the interface of EVERY
object just feels so wrong.

Here we have opposing forces:
 - The first approach adds two methods to your worker class,
   and no methods to any system class.  Instead, it uses
   "does this object know how to iterate over its elements?"
   and "is this object nil?".  This does minimal damage to
   system structure at the price of a little ritual impurity.

 - The second approach adds a method to four system classes,
   enlarging the interface of every object in the system,
   creating a level of coupling we'd be better without, and
   making it harder for someone reading your code to figure
   out what's going on.

In the context of a Composite, with a limited number of
application classes involved, the second approach is better;
the method is/methods are not defined anywhere they should
not be.

In the context of *this* problem, the first approach is
better.  MUCH better.  Why?  Because *encapsulation* is much
more important to OOP than absence-of-IFs.

On Sun, 13 Sep 2020 at 21:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof



--------------------------------------------
Stéphane Ducasse
03 59 35 87 52
Assistant: Aurore Dalle 
FAX 03 59 57 78 50
TEL 03 59 35 86 16
S. Ducasse - Inria
40, avenue Halley, 
Parc Scientifique de la Haute Borne, Bât.A, Park Plaza
Villeneuve d'Ascq 59650
France

Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Richard O'Keefe
In a language like Java or C# or Ada we can insist at
compile time that an argument conforms to a protocol,
whatever class it might be an instance of.

In Smalltalk, we can use #isKindOf: to test whether
an object inherits from a class, but that is often the
wrong test.  We can use (multiple calls to) #respondsTo:
to tell whether an object conforms to a protocol.

The only snag, of course, is that in too many Smalltalk
systems, #respondsTo: is not trustworthy.  If a class
has a method that sends #subclassResponsibility or
#shouldNotImplement, it's still counted as responding
to the selector in question.

This is not a problem in my Smalltalk because
(a) you are not allowed to create a direct instance of
a class where any method calls #subclassResponsibility.
(b) #shouldNotImplement appears only in test cases.
Since I already have named protocols, with classes
declaring their intention to conform to them, I've
been thinking about adding
  aClass implements: aProtocol
  anInstance conformsTo: aProtocol
but have not done that yet.

I note that Pharo V7.0.3 had 132 senders of #respondsTo:
and Pharo V8.0.0 has 126 senders of #respondsTo:.
These neatly bracket Dolphin 7's 130.



On Wed, 16 Sep 2020 at 06:50, Stéphane Ducasse <[hidden email]> wrote:


On 14 Sep 2020, at 16:02, Roelof Wobben via Pharo-users <[hidden email]> wrote:


Thanks Richard for this explanation.
and I like also your idea of using #respondTo instead of asking a object if its a collection or a item.

refrain from using respondTo:
I make code difficult to evolve and can introduce vicious bugs.

S. 
Roelof


Op 14-9-2020 om 14:28 schreef Richard O'Keefe:
"OOP is not asking an object what it is"?  You've lost me.
I'm reminded of a joke exam question that goes something
like this:
   Some things have ping nature and other things have pong nature.
   Discuss, with examples.

Whatever else it is, OOP is a means to an end, not an end in itself.
It's not a religion.  Allah will not cast you into the Fire for using
something which is not ritually pure.

The whole point of the Design Patterns movement was not to present
canned solutions but to describe common *situations* characterised
by (metaphorical) *forces* pushing you in incompatible directions.

Question 1: Why do you even have a stream there?
flattenArray: aCollection
    |buffer|
    buffer := OrderedCollection new: aCollection size.
    self flattenArray: aCollection into: buffer.
    ^buffer asArray

flattenArray: anObject into: buffer
    (anObject respondsTo: #do:)
       ifTrue:  [anObject do: [:each | self flattenArray: each into: buffer]
       ifFalse: [anObject ifNotNil: [buffer addLast: anObject].

(CAUTION: untested code.)

Question 2: is there any point in *trying* to make this more ping and less pong?
I have to say that in 40 years of programming, I have *never* wanted to flatten
a completely arbitrary structure.  When I have wanted to flatten something, it
has always been an instance of the Composite design pattern, so that a specific
and quite small set of classes has been involved.

I am well aware that some Smalltalk systems have some sort of 'flatten'
lying around like a rake in the grass, but I have found no two which agree
on the specification.

Question 3: Let's consider an if-less alternative.

Stream
  flattenInto: buffer
    self do: [:each| each flattenInto: buffer].

Collection
  flattenInto: buffer
    self do:[:each | each flattenInto: buffer].

Object
  flattenInto: buffer
    buffer addLast: self.

UndefinedObject
  flattenInto: buffer
    "Do nothing."

This is Smalltalk, where we *can* add methods to system classes like these.
When the payoff is worth it, I have no qualms whatever in doing so.
But putting methods of little or no utility into the interface of EVERY
object just feels so wrong.

Here we have opposing forces:
 - The first approach adds two methods to your worker class,
   and no methods to any system class.  Instead, it uses
   "does this object know how to iterate over its elements?"
   and "is this object nil?".  This does minimal damage to
   system structure at the price of a little ritual impurity.

 - The second approach adds a method to four system classes,
   enlarging the interface of every object in the system,
   creating a level of coupling we'd be better without, and
   making it harder for someone reading your code to figure
   out what's going on.

In the context of a Composite, with a limited number of
application classes involved, the second approach is better;
the method is/methods are not defined anywhere they should
not be.

In the context of *this* problem, the first approach is
better.  MUCH better.  Why?  Because *encapsulation* is much
more important to OOP than absence-of-IFs.

On Sun, 13 Sep 2020 at 21:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof



--------------------------------------------
Stéphane Ducasse
03 59 35 87 52
Assistant: Aurore Dalle 
FAX 03 59 57 78 50
TEL 03 59 35 86 16
S. Ducasse - Inria
40, avenue Halley, 
Parc Scientifique de la Haute Borne, Bât.A, Park Plaza
Villeneuve d'Ascq 59650
France

Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Stéphane Ducasse
still my point holds.
And I’m like saint christophe I only believe what I saw and we will never see what you are doing so it does not exist. 
This is the rule of open-source. 

S.

On 16 Sep 2020, at 04:51, Richard O'Keefe <[hidden email]> wrote:

In a language like Java or C# or Ada we can insist at
compile time that an argument conforms to a protocol,
whatever class it might be an instance of.

In Smalltalk, we can use #isKindOf: to test whether
an object inherits from a class, but that is often the
wrong test.  We can use (multiple calls to) #respondsTo:
to tell whether an object conforms to a protocol.

The only snag, of course, is that in too many Smalltalk
systems, #respondsTo: is not trustworthy.  If a class
has a method that sends #subclassResponsibility or
#shouldNotImplement, it's still counted as responding
to the selector in question.

This is not a problem in my Smalltalk because
(a) you are not allowed to create a direct instance of
a class where any method calls #subclassResponsibility.
(b) #shouldNotImplement appears only in test cases.
Since I already have named protocols, with classes
declaring their intention to conform to them, I've
been thinking about adding
  aClass implements: aProtocol
  anInstance conformsTo: aProtocol
but have not done that yet.

I note that Pharo V7.0.3 had 132 senders of #respondsTo:
and Pharo V8.0.0 has 126 senders of #respondsTo:.
These neatly bracket Dolphin 7's 130.



On Wed, 16 Sep 2020 at 06:50, Stéphane Ducasse <[hidden email]> wrote:


On 14 Sep 2020, at 16:02, Roelof Wobben via Pharo-users <[hidden email]> wrote:


Thanks Richard for this explanation.
and I like also your idea of using #respondTo instead of asking a object if its a collection or a item.

refrain from using respondTo:
I make code difficult to evolve and can introduce vicious bugs.

S. 
Roelof


Op 14-9-2020 om 14:28 schreef Richard O'Keefe:
"OOP is not asking an object what it is"?  You've lost me.
I'm reminded of a joke exam question that goes something
like this:
   Some things have ping nature and other things have pong nature.
   Discuss, with examples.

Whatever else it is, OOP is a means to an end, not an end in itself.
It's not a religion.  Allah will not cast you into the Fire for using
something which is not ritually pure.

The whole point of the Design Patterns movement was not to present
canned solutions but to describe common *situations* characterised
by (metaphorical) *forces* pushing you in incompatible directions.

Question 1: Why do you even have a stream there?
flattenArray: aCollection
    |buffer|
    buffer := OrderedCollection new: aCollection size.
    self flattenArray: aCollection into: buffer.
    ^buffer asArray

flattenArray: anObject into: buffer
    (anObject respondsTo: #do:)
       ifTrue:  [anObject do: [:each | self flattenArray: each into: buffer]
       ifFalse: [anObject ifNotNil: [buffer addLast: anObject].

(CAUTION: untested code.)

Question 2: is there any point in *trying* to make this more ping and less pong?
I have to say that in 40 years of programming, I have *never* wanted to flatten
a completely arbitrary structure.  When I have wanted to flatten something, it
has always been an instance of the Composite design pattern, so that a specific
and quite small set of classes has been involved.

I am well aware that some Smalltalk systems have some sort of 'flatten'
lying around like a rake in the grass, but I have found no two which agree
on the specification.

Question 3: Let's consider an if-less alternative.

Stream
  flattenInto: buffer
    self do: [:each| each flattenInto: buffer].

Collection
  flattenInto: buffer
    self do:[:each | each flattenInto: buffer].

Object
  flattenInto: buffer
    buffer addLast: self.

UndefinedObject
  flattenInto: buffer
    "Do nothing."

This is Smalltalk, where we *can* add methods to system classes like these.
When the payoff is worth it, I have no qualms whatever in doing so.
But putting methods of little or no utility into the interface of EVERY
object just feels so wrong.

Here we have opposing forces:
 - The first approach adds two methods to your worker class,
   and no methods to any system class.  Instead, it uses
   "does this object know how to iterate over its elements?"
   and "is this object nil?".  This does minimal damage to
   system structure at the price of a little ritual impurity.

 - The second approach adds a method to four system classes,
   enlarging the interface of every object in the system,
   creating a level of coupling we'd be better without, and
   making it harder for someone reading your code to figure
   out what's going on.

In the context of a Composite, with a limited number of
application classes involved, the second approach is better;
the method is/methods are not defined anywhere they should
not be.

In the context of *this* problem, the first approach is
better.  MUCH better.  Why?  Because *encapsulation* is much
more important to OOP than absence-of-IFs.

On Sun, 13 Sep 2020 at 21:26, Roelof Wobben via Pharo-users <[hidden email]> wrote:
Hello,

I know that OOP is not asking a object what it is but I have to flatten a array so I did this :

flattenArray: aCollection
    ^ (OrderedCollection
        streamContents: [ :stream | self flatten: aCollection into: stream ])
        asArray

flatten: anObject into: result
    ^ anObject isCollection
        ifTrue: [ anObject do: [ :item | self flatten: item into: result ] ]
        ifFalse: [ anObject ifNotNil: [ result nextPut: anObject ] ]

The name flattenArray is given bij exercism. 

Now I wonder how I can make this more a OOP solution. 

Can someone give me some hints or some examples ?

Roelof



--------------------------------------------
Stéphane Ducasse
03 59 35 87 52
Assistant: Aurore Dalle 
FAX 03 59 57 78 50
TEL 03 59 35 86 16
S. Ducasse - Inria
40, avenue Halley, 
Parc Scientifique de la Haute Borne, Bât.A, Park Plaza
Villeneuve d'Ascq 59650
France


--------------------------------------------
Stéphane Ducasse
03 59 35 87 52
Assistant: Aurore Dalle 
FAX 03 59 57 78 50
TEL 03 59 35 86 16
S. Ducasse - Inria
40, avenue Halley, 
Parc Scientifique de la Haute Borne, Bât.A, Park Plaza
Villeneuve d'Ascq 59650
France

Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Sean P. DeNigris
Administrator
In reply to this post by Pharo Smalltalk Users mailing list

Stéphane Ducasse <[hidden email]> wrote:

refrain from using respondTo: I make code difficult to evolve and can introduce vicious bugs.

Steph, would you say more about this? It’s something I’ve been wondering about.

I was recently reading the Strategy pattern in the Smalltalk Companion to the GOF book. On p. 339, they proposed the following "Smalltalk way" as an alternative to the common class-per-strategy implementation:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

| selector |

"Construct the name of the method to invoke:"

selector := ('formatWith', formattingStrategy, 'Algorithm') asSymbol.

self perform: selector

]]]

It then dismissed the approach as "''clever but difficult from a program understanding perspective. Even static analysis tools such as code browsers' "senders" and "messages" fail on this code.''"

It struck me as perhaps a bit extreme (i.e. too clever indeed) to construct the algorithm selector via string concatenation. Maybe "senders" search capabilities have gotten more sophisticated since publication, but Pharo seems to support symbol arguments, even for e.g. message renames. I thought, why not the following:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

self perform: self formattingStrategy

]]]

Then client code like ==aComposition formattingStrategy: #formatWithSimpleAlgorithm== would show up in senders, message renames, etc.

But from what you’re saying, I feel I may be missing something…


Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Richard Sargent
Administrator
On Wed, Sep 16, 2020 at 10:28 AM <[hidden email]> wrote:

Stéphane Ducasse <[hidden email]> wrote:

refrain from using respondTo: I make code difficult to evolve and can introduce vicious bugs.

Steph, would you say more about this? It’s something I’ve been wondering about.

I was recently reading the Strategy pattern in the Smalltalk Companion to the GOF book. On p. 339, they proposed the following "Smalltalk way" as an alternative to the common class-per-strategy implementation:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

| selector |

"Construct the name of the method to invoke:"

selector := ('formatWith', formattingStrategy, 'Algorithm') asSymbol.

self perform: selector

]]]

It then dismissed the approach as "''clever but difficult from a program understanding perspective. Even static analysis tools such as code browsers' "senders" and "messages" fail on this code.''"

It is always best to have senders of the methods you use. Additionally, it's far better to properly manage the user-friendly identification of a thing and the thing itself. e.g. Plain language name mapped to a selector or a block.

Remember, "the fact that one *can* do something is far different from whether one *should* do that thing." (I have suffered far too often and far too long from clever programs preferring the former without thinking about the latter.)

It struck me as perhaps a bit extreme (i.e. too clever indeed) to construct the algorithm selector via string concatenation. Maybe "senders" search capabilities have gotten more sophisticated since publication, but Pharo seems to support symbol arguments, even for e.g. message renames. I thought, why not the following:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

self perform: self formattingStrategy

]]]

Then client code like ==aComposition formattingStrategy: #formatWithSimpleAlgorithm== would show up in senders, message renames, etc.

But from what you’re saying, I feel I may be missing something…


Reply | Threaded
Open this post in threaded view
|

Re: How can I make this more OOP

Stéphane Ducasse
In reply to this post by Sean P. DeNigris
respondTo: is different from a perform based on forging a selector. 
respondTo: implies a different control while perform: is just sending a message. 

Some logic of Spec or something else was doing respondTo: and it is an indication of weak polymorphism.
Either objects are polymorphic or they are not. 
Because else you expect a behavior and because of a hidden respondTo: you get a api not invoked. 
This forces you to read all the code and step to find the super cool users of respondTo: 

So I really stand on my claim respondTo: should only be used in last resort. 
It covers shitty design. 

S. 

On 16 Sep 2020, at 19:28, [hidden email] wrote:

Stéphane Ducasse <[hidden email]> wrote:

refrain from using respondTo: I make code difficult to evolve and can introduce vicious bugs.

Steph, would you say more about this? It’s something I’ve been wondering about.

I was recently reading the Strategy pattern in the Smalltalk Companion to the GOF book. On p. 339, they proposed the following "Smalltalk way" as an alternative to the common class-per-strategy implementation:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

| selector |

"Construct the name of the method to invoke:"

selector := ('formatWith', formattingStrategy, 'Algorithm') asSymbol.

self perform: selector

]]]

It then dismissed the approach as "''clever but difficult from a program understanding perspective. Even static analysis tools such as code browsers' "senders" and "messages" fail on this code.''"

It struck me as perhaps a bit extreme (i.e. too clever indeed) to construct the algorithm selector via string concatenation. Maybe "senders" search capabilities have gotten more sophisticated since publication, but Pharo seems to support symbol arguments, even for e.g. message renames. I thought, why not the following:

[[[language=smalltalk

Composition>>repair

"Without the strategy pattern, but using perform:."

self perform: self formattingStrategy

]]]

Then client code like ==aComposition formattingStrategy: #formatWithSimpleAlgorithm== would show up in senders, message renames, etc.

But from what you’re saying, I feel I may be missing something…



--------------------------------------------
Stéphane Ducasse
03 59 35 87 52
Assistant: Aurore Dalle 
FAX 03 59 57 78 50
TEL 03 59 35 86 16
S. Ducasse - Inria
40, avenue Halley, 
Parc Scientifique de la Haute Borne, Bât.A, Park Plaza
Villeneuve d'Ascq 59650
France