Issue 3436 in pharo: #copyFrom: is broken

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

Re: Issue 3436 in pharo: #copyFrom: is broken

pharo

Comment #18 on issue 3436 by cesar.rabak: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

Your comments show consistency with your vision stated above about the need  
of an overhaul on the design of the interface (a.k.a. "API"), if you take  
the bulk of it.

In order to sustain a civilized dialog, I need to concede you with this  
doubt.  However, it seems to me this is flaring the judgment of proposal  
here:

The method name has *copy* in it, hasn't?  I _does_ return a *copy* of the  
object, doesn't?

This issue #3436 is about: is it more adequate that a method  
called "copyFrom:" copies the state of an object or a part of one when it  
is a SequenceableCollection?

Your other points have to be addressed elsewhere, be it in the mailing list  
as a preliminary discussion or as proposal here via a well formed issue.

In order to avoid any doubts, my stance is that we adopt copyFrom: as an  
alias (or 'short cut') to "#copyFrom: start to: (self size)" in the most  
appropriate point of the Pharo object's hierarchy.



Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

pharo

Comment #19 on issue 3436 by [hidden email]: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

Nicolas wrote:
> All the other messages could have had the copy prepended if it was a
> question of homogeneitty with existing API.
> The fact is that they were added later than copy and copyFrom:to:
> Of course, we can as well remove the copy prefix, but do we really care ?
> Is it really what makes the message ambiguous ?

It is one of the ambigueties.

> If I wouldn't know about the message, my question would not be about
> whether or not the message triggers a copy.
> It would be whether the argument are elements or indices.

- does it return a copy or change self
- does it return a collection or an element
- are arguments elements or indices

should be reflected in the naming of the method, if one wants to avoid  
having to read the implementation/comments.

Currently, every method having copy in its name has to be examined closely,  
to be able to decide what behavior should be expected. Overriding semantics  
is less than helpful, as that leads to more confusion. As long as there is  
no clear naming convention, it is better to add new method names than to  
overlay new behavior on existing ones.


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

csrabak
In reply to this post by Nicolas Cellier
Em 07/01/2011 15:10, Nicolas Cellier < [hidden email] > escreveu:

You make excellent points Nicolas!


> All the other messages could have had the copy prepended if it was a
> question of homogeneitty  with existing API.  The fact  is that they
> were added  later than  copy and copyFrom:to:
>  Of course, we  can as well remove the  copy prefix, but do we really care  ?  

I think we should, as the removal would have ripple effects in production code.
Our reasoning should be some sort of ROI on it.

> Is it really what makes the message ambiguous ?

I would argue that the presence of 'copy' suffix makes them _less_ ambiguous as
far as the methods in question effectively do a copy on the receiver.

>  If  I wouldn't know  about the  message, my  question would  not be
> about  whether or  not the  message triggers  a copy.   It  would be
> whether the argument are elements or indices.

You mean that once you have removed the 'copy' prefix or as the method names stand right now (and I mean for the ones this issue number is about)?

>  I could expect:
>  #( 'foo' 'bar' 'baz') copyFrom: 'bar' to: 'baz'.
>  to work like:
>  #( 'foo' 'bar' 'baz') after: 'foo'.

I would like to elaborate on your half million question « Is it really what makes the message ambiguous ? »
 
If you attempt your example in plain Pharo image

#( 'foo' 'bar' 'baz') copyFrom: 'bar' to: 'baz'.

You get a MNU

ByteString(Object)>>doesNotUnderstand: #-
Array(SequenceableCollection)>>copyFrom:to:
.
.
.

So what would be your point:

1) copyFrom:to: is not 'polymorphic enough' for Pharo Smalltalk ideal of a dynamic language?

2) the message name is to be changed to copyFromIndex: start toIndex: end?

3) some other concoction I missed?


HTH

--
Cesar Rabak

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

csrabak
In reply to this post by pharo
Em 07/01/2011 16:43, [hidden email] escreveu:

>  Comment #19 on issue 3436 by [hidden email]: #copyFrom: is broken
> http://code.google.com/p/pharo/issues/detail?id=3436
>  Nicolas wrote:
> > All the other messages could have had the copy prepended if it was
> > a question  of homogeneitty with  existing API.  The fact  is that
> > they were added later than copy and copyFrom:to: Of course, we can
> > as well  remove the copy  prefix, but do  we really care ?   Is it
> > really what makes the message ambiguous ?

>  It is one of the ambigueties.

The ambiguity is of course a thing we have to qualify.  The Collection
hierarchy of Smalltalk uses Integers for indices for accessing elements.

This is something you need to learn to be able to use these classes.

>
> > If I  wouldn't know  about the message,  my question would  not be
> > about whether  or not  the message triggers  a copy.  It  would be
> > whether the argument are elements or indices.
>
> - does it return  a copy or change self does  it return a collection

Well the word 'copy' seems to indicate the it doesn't change self, if it
did, we should open another issue and search for a fix.  The other part of
the enthymeme of yours must be pointed out to practicality and the sensible
thing to do.  The return would most of the time be the most appropriate
collection given the intent of the extraction done by the method.

Pathological cases should be issued and discussed for a fix.

> - or an element are arguments elements or indices

There is a limit on the 'non knowledge' you can [not] have in order to use
the methods of a determined class.  In Pharo you have as a last recourse
the browsing of the method implementation itself.


>  should be  reflected in the naming  of the method, if  one wants to
> avoid having to read the implementation/comments.

There should be a balance between the three. Otherwise you'll end up with
too verbose API ("interface") for doing the daily chores, and people will
frown and stop using the language.  Even with OCompletion and friends, the
most common operations should strive to have short method names and be relatively
mnemonic for people who get trained in the environment/technology/language.


>  Currently, every method having copy  in its name has to be examined
> closely,   to  be   able   to  decide   what   behavior  should   be
> expected.

Once we get evidence on this we could proceed. Do you have any case to
show us?
 
> Overriding  semantics is less than helpful,  as that leads
> to more confusion.  As long as there is  no clear naming convention,
> it is better to add new method names than to overlay new behavior on
> existing ones.

No. We have a history and production code to live with as well.  In extremis
your suggestion is better accomplished with a proposal for a new, parallel,
hierarchy of classes with a 'better' interface, which put in the ecosystem
should show if its benefits make developers to change to it instead of the
present one.

my 0.019999...

--
Cesar Rabak


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

Nicolas Cellier
In reply to this post by csrabak
2011/1/7  <[hidden email]>:

> Em 07/01/2011 15:10, Nicolas Cellier < [hidden email] > escreveu:
>
> You make excellent points Nicolas!
>
>
>> All the other messages could have had the copy prepended if it was a
>> question of homogeneitty  with existing API.  The fact  is that they
>> were added  later than  copy and copyFrom:to:
>>  Of course, we  can as well remove the  copy prefix, but do we really care  ?
>
> I think we should, as the removal would have ripple effects in production code.
> Our reasoning should be some sort of ROI on it.
>
>> Is it really what makes the message ambiguous ?
>
> I would argue that the presence of 'copy' suffix makes them _less_ ambiguous as
> far as the methods in question effectively do a copy on the receiver.
>
>>  If  I wouldn't know  about the  message, my  question would  not be
>> about  whether or  not the  message triggers  a copy.   It  would be
>> whether the argument are elements or indices.
>
> You mean that once you have removed the 'copy' prefix or as the method names stand right now (and I mean for the ones this issue number is about)?
>
>>  I could expect:
>>  #( 'foo' 'bar' 'baz') copyFrom: 'bar' to: 'baz'.
>>  to work like:
>>  #( 'foo' 'bar' 'baz') after: 'foo'.
>
> I would like to elaborate on your half million question « Is it really what makes the message ambiguous ? »
>
> If you attempt your example in plain Pharo image
>
> #( 'foo' 'bar' 'baz') copyFrom: 'bar' to: 'baz'.
>
> You get a MNU
>
> ByteString(Object)>>doesNotUnderstand: #-
> Array(SequenceableCollection)>>copyFrom:to:
> .
> .
> .
>
> So what would be your point:
>
> 1) copyFrom:to: is not 'polymorphic enough' for Pharo Smalltalk ideal of a dynamic language?
>
> 2) the message name is to be changed to copyFromIndex: start toIndex: end?
>
> 3) some other concoction I missed?
>
>
> HTH
>
> --
> Cesar Rabak
>
>

Here is what I mean:
I agree that #copyFrom: is ambiguous and could be renamed, with all
deprecation precautions.

But concerning #copyFrom:to: I think this name is a non issue for code
readability.
I decided to play advocatus diaboli just to push a bit absurdness of
this ubuesque thread.
In Smalltalk, you have to trust what you read, and to me (aString
copyFrom: 2 to: aString size - 3) is obviously a copy, and arguments
are obviously indices, what else could it be ?
So no, I don't suggest to drop the copy though it's not strictly
necessary, nor to append an Index to the name.
Who never browsed senders of such message in squeaksource code base
could have such a foolish idea.

For writing code, we can't just guess an API, so this is a non issue too.
I can't guess the name, it could be #copyFromStartIndex:toStopIndex:
or just #from:to:, no way the API will fit everyone's brain.
For writing code we have these alternatives:
- Open a Browser and browse the collection classes
- learn idioms from reading other code
- learn idioms from a tutorial or a help system
- type a keyword in a message finder, or some receiver arguments and
expected results.
All these actions will clearly disambiguate the semantic of
copyFrom:to: arguments.
We can tolerate some level of ambiguity when the send sites will disambiguate.

As for homogeneity, I don't propose anything but statu quo.
I understand (aString first: 3) as well as extending (aString first),
and I can reasonnably guess it will answer a copy too, why the hell
would this innocent message change aString state ? Does (aString
first) do ?
So no, I don't need to add a copy prefix to this message, and no i
don't really care for a higher homogeneity.

If we change a few missnamed selectors here and there, that's OK, but
if we change a large amount of Kernel messages with many senders, it's
a recipee for Pharo rejection, because we would make developer's life
a hell for no valid reason.

Nicolas

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

csrabak
Em 07/01/2011 18:17, Nicolas Cellier < [hidden email] > escreveu:
2011/1/7  :
[snipped]

> > I would  like to elaborate on  your half million question  « Is it
> > really what makes the message ambiguous ? »
> > If you attempt your example in plain Pharo image
> >
> > #( 'foo' 'bar' 'baz') copyFrom: 'bar' to: 'baz'.
> > You get a MNU
> > ByteString(Object)>>doesNotUnderstand:                           #-
> > Array(SequenceableCollection)>>copyFrom:to: .  .  .
> > So what would be your point:
> > 1) copyFrom:to:  is not  'polymorphic enough' for  Pharo Smalltalk
> > ideal of a dynamic language?
> > 2)  the message  name is  to  be changed  to copyFromIndex:  start
> > toIndex: end?
> > 3) some other concoction I missed?

>  Here is what I mean: I agree that #copyFrom: is ambiguous and could
> be renamed, with all deprecation precautions.

I will take this as meaning "I agree that Object>>copyFrom: is ambiguous..."

If I misunderstood you, please feel free to correct.

OTHO, as it is not explicitly written, I would like to understand your
stance on having this method name as a shorcut to
copyFrom: start to: (self size).

>  But concerning #copyFrom:to:  I think this name is  a non issue for
> code readability.  

Which I also agree, and add that it helps to keep the API as unambiguous
as it gets.

> I decided to  play advocatus diaboli just to push
> a bit absurdness of this ubuesque thread.  

It may sound like because we're thinking only in our crib space.  The OP
brought a point which converges with other threads in this mailing list
about the difficulties an [Smalltalk] aficionado incurs when trying to show
Smalltalk to people who already know present technology languages and
environments.

> In Smalltalk, you have to
> trust what you read, and to me (aString copyFrom: 2 to: aString size
> - 3) is obviously a  copy, and arguments are obviously indices, what
> else could it be  ?

I do agree 100% on this.  However, there is a rub:

aComplicatedName copyFrom: pocnu to: kraja.

What can they be?  As Smalltalk is dynamic even if you could see the

|pocnu kraja|
.
.
.
you'll probably would need to see the implementers of the code that attribute
values to these (say) local variables.

And to avoid lengthy and unnecessary follow ups on this: I don't see any problem
on this, because Smalltalk (and Pharo in particular) makes this easy enough to do.
 

> So no, I don't suggest to  drop the copy though
> it's not  strictly necessary,  nor to append  an Index to  the name.

Great! ;-)

> Who never browsed senders of  such message in squeaksource code base
> could have such a foolish idea.

Again I think this is a reasoning that only can be explained due being
too comfy in our crib:

Put this stub of code in a book (especially if the dead tree variety) and
claim that it is only a 'browsing of sender' away and you will understand
WHY the whole thread is not that much "ubuesque" as it seemed at first...

>  For writing  code, we  can't just guess  an API,  so this is  a non
> issue   too.     I   can't   guess    the   name,   it    could   be
> #copyFromStartIndex:toStopIndex: or  just #from:to:, no  way the API
> will  fit  everyone's  brain.  

Right we should not guess, but it is not the whole story. There is some
ergonomics  in a well designed interface of the APIas put in the issue
tracker.

Names should be as consistent as possible and be concurrent to present
programming practices.  This will ease the way people learn and think
in the programming language.
 
> For  writing  code  we  have  these
> alternatives:
> - Open a Browser and browse the collection classes learn idioms from
> - reading other code  learn idioms from a tutorial  or a help system
> - type a keyword in a message finder, or some receiver arguments and
> expected results.   All these actions will  clearly disambiguate the
> semantic of  copyFrom:to: arguments.  We can tolerate  some level of
> ambiguity when the send sites will disambiguate.

The semantics of copyFrom:to: are not the issue. The issue as is written
in the issue tracker _and_ the subject line is the semantics of copyFrom:

>  As  for homogeneity,  I don't  propose anything  but statu  quo.  I
> understand (aString first: 3)  as well as extending (aString first),
> and I can reasonnably guess it  will answer a copy too, why the hell
> would  this innocent message  change aString  state ?  

I agree with the later, but not with the former: could aString first
return a String or Character?

> Does (aString first)  do ?

>   So no,  I don't  need to  add a  copy prefix  to this
> message, and no i don't really care for a higher homogeneity.
>  If we change  a few missnamed selectors here  and there, that's OK,
> but  if  we change  a  large amount  of  Kernel  messages with  many
> senders, it's a  recipee for Pharo rejection, because  we would make
> developer's life a hell for no valid reason.

Or not! There is no reason why a newer "kernel" be created and if the usage
of it be so compelling or productive or practical or ....<put your favorite
marketing word> that it takes over the present state of affairs in the
usage of Smalltalk, why not!?

--
Cesar Rabak

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

Nicolas Cellier
2011/1/7  <[hidden email]>:
>
> I will take this as meaning "I agree that Object>>copyFrom: is ambiguous..."
>
> If I misunderstood you, please feel free to correct.
>
> OTHO, as it is not explicitly written, I would like to understand your
> stance on having this method name as a shorcut to
> copyFrom: start to: (self size).
>

I don't think it's a good idea for backward compatibility reasons.
Changing the semantics of #copyFrom: is worse than just deprecating the message.
Do we really need this message ? For porting a package from another dialect ?

>
> I do agree 100% on this.  However, there is a rub:
>
> aComplicatedName copyFrom: pocnu to: kraja.
>
> What can they be?  As Smalltalk is dynamic even if you could see the
>
> |pocnu kraja|
> .
> .
> .
> you'll probably would need to see the implementers of the code that attribute
> values to these (say) local variables.
>

Of course, one can always obfuscate its code, and the naming of these
variable is well chosen with this respect.

>
> Again I think this is a reasoning that only can be explained due being
> too comfy in our crib:
>
> Put this stub of code in a book (especially if the dead tree variety) and
> claim that it is only a 'browsing of sender' away and you will understand
> WHY the whole thread is not that much "ubuesque" as it seemed at first...
>

We can perfectly write a book explaining these selectors.
Just group the selectors (copy copyFrom:to: etc..), explain copy vs
partial copy, and explain theses selectors are in the ANSI standard
API.
Then group the selectors (first, first: etc..), explain that they are
extensions, and don't need a copy prefix, because anyway most messages
will do a copy.

>
> The semantics of copyFrom:to: are not the issue. The issue as is written
> in the issue tracker _and_ the subject line is the semantics of copyFrom:
>

Agree, but i was seeing this thread derivate a bit.

>>   So no,  I don't  need to  add a  copy prefix  to this
>> message, and no i don't really care for a higher homogeneity.
>>  If we change  a few missnamed selectors here  and there, that's OK,
>> but  if  we change  a  large amount  of  Kernel  messages with  many
>> senders, it's a  recipee for Pharo rejection, because  we would make
>> developer's life a hell for no valid reason.
>
> Or not! There is no reason why a newer "kernel" be created and if the usage
> of it be so compelling or productive or practical or ....<put your favorite
> marketing word> that it takes over the present state of affairs in the
> usage of Smalltalk, why not!?
>

Well, we have to balance pros and cons of changing, especially in the kernel.

> --
> Cesar Rabak
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

Stéphane Ducasse
What we will do is

        deprecate copyFrom: from Object -> copyStateFrom:
        add copyFrom: in Sequenceable
                        as
                        self copyFrom: a to: self size

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

Peter Hugosson-Miller
+100 :-D

--
Cheers,
Peter

On Sat, Jan 8, 2011 at 3:22 PM, Stéphane Ducasse <[hidden email]> wrote:
What we will do is

       deprecate copyFrom: from Object -> copyStateFrom:
       add copyFrom: in Sequenceable
                       as
                       self copyFrom: a to: self size

Stef


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

csrabak
In reply to this post by Stéphane Ducasse
+1

If you accept a two phase approach (as I understood Seaside may be heavily impacted):

1)   Let's add add copyFrom: in Sequenceable  as  self copyFrom: a to: self size;
1.1) Test for issues with Seaside/Mondrian, etc., i.e., all production environments, if
     they come green from this mod, commit.
2)   Do the deprecation protocol in Object>>copyFrom: (adding Object>>copyStateFrom:).
3)   Remove Object>>copyFrom:

Regards,

--
Cesar Rabak


Em 08/01/2011 12:22, Stéphane Ducasse < [hidden email] > escreveu:
What we will do is

 deprecate copyFrom: from Object -> copyStateFrom:
 add copyFrom: in Sequenceable
 as
 self copyFrom: a to: self size

Stef



Reply | Threaded
Open this post in threaded view
|

Re: Issue 3436 in pharo: #copyFrom: is broken

pharo
In reply to this post by pharo
Updates:
        Labels: -Milestone-1.3

Comment #20 on issue 3436 by [hidden email]: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

(No comment was entered for this change.)


12