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) almost 11 years ago
          Updated by marcandre (Marc-Andre Lafortune) almost 11 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) almost 11 years ago
          Updated by sawa (Tsuyoshi Sawada) almost 11 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) almost 11 years ago
          Updated by fra.boffa (Francesco Boffa) almost 11 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) almost 11 years ago
          Updated by marcandre (Marc-Andre Lafortune) almost 11 years ago
          
          
        
        
      
      Francesco Boffa wrote:
I insist, however, that this should at least be made clearer in the documentation of all the
to_smethods 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.