Bug #10795
closedto_s returns references to self if called on string
Description
This is not actually a bug, but rather a strong violation of the Principle of least astonishment.
Lately I found a bug in the Rails project I work to. There was a method concatenating two float values with a comma, converting them to string like this:
def coords_to_string
latitude.to_s << "," << longitude.to_s
end
The bug happened when we passed latitude and longitude as strings instead of floats.
Actually, 5.12345.to_s
returns a new string object and any operation made on that string wont affect the original 5.12345. Instead "5.12345".to_s
will return the same instance of that string, so that the <<
method will affect the original string.
So we found that our latitude variable was growing on each call and after a while it become "5.12345,13.12345,13.12345,13.12345,13.12345,13.12345"
This was probably made like this for performance reasons, however we found it a clear violation of the POLA, whereas we expected to_s to always return a new instance of that string.
Hope, this may help.
Francesco Boffa
Updated by marcandre (Marc-Andre Lafortune) about 9 years ago
- Status changed from Open to Rejected
Remember, the principle of least astonishment applies to Matz only :-)
Changing this would, among other things, be a source of incompatibilities.
In any case, your code is better written
def coords_to_string
"#{latitude},#{longitude}"
end
Updated by sawa (Tsuyoshi Sawada) about 9 years ago
Marc-Andre Lafortune wrote:
In any case, your code is better written
def coords_to_string "#{latitude},#{longitude}" end
or
def coords_to_string
[latitude, longitude].join(",")
end
Updated by fra.boffa (Francesco Boffa) about 9 years ago
For sure, our code was not the ideal for that task, and indeed, I already had changed it to one of your sane alternatives and the original committer has been appropriately thrown by the window :).
I insist, however, that this should at least be made clearer in the documentation of all the to_s
methods available in the standard library.
Thanks for your attention.
Updated by marcandre (Marc-Andre Lafortune) about 9 years ago
Francesco Boffa wrote:
I insist, however, that this should at least be made clearer in the documentation of all the
to_s
methods available in the standard library.
Not sure what you mean by "all" to_s
methods. Only String#to_s
returns the receiver.
The document is quite explicit, both in the interface and the description. OTOH, it didn't mention the effect for subclasses, so I modified it to follow that of Array#to_a
.