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
|

Issue 3436 in pharo: #copyFrom: is broken

pharo
Status: New
Owner: ----

New issue 3436 by sebastianconcept: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

1.2 Unstable version 12272

Steps to reproduce:
1.
2.
3.

Just do cmd-p on this:

('some shit' copyFrom: 5) =  'some shit' ifTrue:['WTF?!']






Reply | Threaded
Open this post in threaded view
|

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

pharo

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

#copyFrom: is not intended to copy strings, it is used to copy instVars  
that are common to the receiver and the argument into the receiver.  
Behaves as expected.  Perhaps the name is misleading ?!?


Reply | Threaded
Open this post in threaded view
|

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

pharo
Updates:
        Status: Invalid

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

Yeah, clearly mixed it up with copyFrom:to:, copyAfter:, or another similar  
collection method.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

I don't get it.

When you take any non squeak based smalltalk you'll see that #copyFrom: is  
consistent and intuitive.

When you take Pharo you get that:
1. it's broken even for smalltalkers (imagine random citizens)
2. is manipulating (admitedly) dangerously (check implementor comment)
3. is doing something extremely primitive without using the prefix basic  
(or whatever that suggest that this is a thing to use unless you know what  
you're doing)
4. our community response is that reporting this is invalid?

Can we please fix this instead of rationalize?

Suggestion: a rename of the current Object>>copyFrom: will be enough. How  
to rename it? with something that suggest is going to manipulate the  
freaking instance variables)
there is code depending on that? screw it. If they started with the wrong  
foot, they should pay the price of their upgrade (they are guilty of  
creating a dependence on something broken).


Reply | Threaded
Open this post in threaded view
|

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

pharo
Updates:
        Status: Accepted
        Labels: Milestone-1.3

Comment #4 on issue 3436 by stephane.ducasse: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

I suggest to use copyStateFrom: and add copyFrom: in String.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

If we end up deprecating and reimplementing, at least make the "new"  
implementation in SequenceableCollection, where it can be found in STX and  
GnuST.

I still don't think it's a good idea to add to the SequenceableCollection  
API though, because:
- It's not ANSI (VW for instance does not have it)
- There is already copyFrom:to: and allButFirst: filling the same role  
nearly as easy
- It'll break code for no better reason than to increase readability  
(unless you actually look up its implementation, in which case it's quite  
clear what it does)
- There's yet another point of difference if you want to have your code  
running on both Squeak and Pharo



Reply | Threaded
Open this post in threaded view
|

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

pharo

Comment #6 on issue 3436 by stephane.ducasse: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

Yes when I said string it was to say close to copyFrom:to:.
Now there are two issues:
  - Object>>copyFrom: -> Object>>copyStateFrom:

and adding copyFrom:
is not so bad. Consistency is good.
Then I do not understand why the Finder did not show up allButFirst:


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

The argument would be one off?
allButFirst: n  == copyFrom: n+1


Reply | Threaded
Open this post in threaded view
|

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

pharo

Comment #8 on issue 3436 by stephane.ducasse: #copyFrom: is broken
http://code.google.com/p/pharo/issues/detail?id=3436

Not really but when you have copyFrom:to: copyFrom: is natural.
To my eyes this looks like a missing method in the collection API.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

Then the real question  is: why do we have copyFrom:to:?
To be in collection protocol style, that could be from:to:.
It is not reliably derivable from the protocol name if you get a copy or  
mutate the original.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

Well.. please ignore who said it, I don't care and you shouldn't be  
distracted with that, but we are looking at the problem right in the eye:
"- It'll break code for no better reason than to increase readability"

I don't recall seeing this so clearly.

Here is the thing:

every time that code usability is considered something "minor"  
or "wasteful" or "those things that stupid people needs" we are validating  
that smalltalk:
1. gives a shit about being intuitive
2. gives a shit about newcomers
3. of course shouldn't be mainstream.

Every time.

Why?

Because if Jhon Noob the new developer can't do it (due to an absolutely  
irrelevant lack of readability) then he will feel excluded and his employer  
frustrated with the results and his boss tempted to blame the people who  
convinced him to work that project on smalltalk nobody but old-school  
people can deal with.

It will not matter what we say or understand about it. Facts will talk by  
themselves in the eyes of other people.

And if your plan is to force other people to understand you, then be honest  
with yourself: you are working to earn the shitty ranking position of your  
piece of software.

So, really? is that what we want?

Usability matters (and yes, not only at UI level).

Deal with it.

If you think that Smalltalk is suposed to be freaking good at that, then  
make it f* happen.

PS: I'm not enjoying to write this cynical thing, but softy feedback is  
largely proven to be a #fail recipe


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

The problem is that it clearly does nothing at all to improve readability  
and usability, as long as there is no consistent and complete interface.  
Without a complete interface redesign, this simply is no improvement.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

"Au contraire" it does a *bit* to improve usability and as a consequence  
the readability.

Blaming the need for a "a complete interface redesign" as a precondition to  
act is an invitation to remain pusillanimous and not change anything  
(a.k.a. "analysis paralisis").

IMO this move is small, but in the right direction.  The main issue is that  
as the Object>>copyFrom: is being heavily used in production code, a  
planned way of doing this change has to be devised.



Reply | Threaded
Open this post in threaded view
|

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

pharo

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

I have to disagree strongly. It is absolutely unclear why the 'copy' is  
part of the method name in copyFrom:to:.
It therefore is a change in the wrong direction.


Reply | Threaded
Open this post in threaded view
|

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

pharo

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

If you want to clarify this issue, you'll need to have access to the  
minutes and meeting notes of NCTIS J20 which in the ANSI Standard for  
Smalltalk have put in "5.7.8.7 Message:  copyFrom: start to: stop" the  
following Synopsis: "Answer a new collection containing all of the elements  
of the receiver between the indices start
and stop inclusive. If stop < start, the result has a size of zero."

What the OP is asking for IMO is for an extension of this where #copyFrom:  
would mean (OK, call it syntactic sugar if you want to) #copyFrom: start  
to: self size.

The standard is old. What other Smalltalk implementations are doing?

Dolphin Smalltalk implements #copyFrom: as above.
ObjectStudio Smalltalk implements #copyFrom: as above.
ST/X Smalltalk implements #copyFrom: as above.
VW Smalltalk implements #copyFrom: as Squeak/Pharo.

I've not access in this machine to other Smalltalks, if anyone could see  
what Instantiations, MT and any other Smalltalks do w.r.t. it would be  
instructive to complete the table above.






Reply | Threaded
Open this post in threaded view
|

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

pharo

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

GnuST  does the copyFrom: to: self size shortcut as well.

VW does not define copyFrom: in Object.
Implementors in the packages I had loaded:
-ProbedCompiledMethod/Blocks do the same.
-GlorpAttributeModel copies some, others only if nil in receiver, and the  
rest not at all.



Reply | Threaded
Open this post in threaded view
|

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

pharo

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

5.7.8.7 does not adequately explain why copy should be part of the method  
name. To me it looks like accidental complexity, and I would like to avoid  
spreading it.


Reply | Threaded
Open this post in threaded view
|

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

Nicolas Cellier
2011/1/7  <[hidden email]>:
>
> Comment #16 on issue 3436 by [hidden email]: #copyFrom: is broken
> http://code.google.com/p/pharo/issues/detail?id=3436
>
> 5.7.8.7 does not adequately explain why copy should be part of the method
> name. To me it looks like accidental complexity, and I would like to avoid
> spreading it.
>

Because you have two ways to copy a string :
- a copy of the full string
- a partial copy from start index to stop index
does this POV makes it clearer ?

Nicolas

Reply | Threaded
Open this post in threaded view
|

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

pharo
In reply to this post by pharo

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

Nicolas wrote:
> Because you have two ways to copy a string :
> - a copy of the full string
> - a partial copy from start index to stop index
> does this POV makes it clearer ?

This does not help, as we also have a.o.
- allButFirst:
- allButLast:
- first:
- last:
-reverse:
- sortBy:

and compare to:
- after:
- before:
- appendTo:
- sort:

and then to:
- copyEmpty

or even to:
- truncateWithElipsisTo:

How am I supposed to know if I change the original vs get a copy?



Reply | Threaded
Open this post in threaded view
|

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

Nicolas Cellier
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 ?

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.

I could expect:

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

to work like:

#( 'foo' 'bar' 'baz') after: 'foo'.

Nicolas

2011/1/7  <[hidden email]>:

>
> Comment #17 on issue 3436 by [hidden email]: #copyFrom: is broken
> http://code.google.com/p/pharo/issues/detail?id=3436
>
> Nicolas wrote:
>>
>> Because you have two ways to copy a string :
>> - a copy of the full string
>> - a partial copy from start index to stop index
>> does this POV makes it clearer ?
>
> This does not help, as we also have a.o.
> - allButFirst:
> - allButLast:
> - first:
> - last:
> -reverse:
> - sortBy:
>
> and compare to:
> - after:
> - before:
> - appendTo:
> - sort:
>
> and then to:
> - copyEmpty
>
> or even to:
> - truncateWithElipsisTo:
>
> How am I supposed to know if I change the original vs get a copy?
>
>
>
>

12