[yast-devel] Equality is important (comparing Array elements issue)
## Short version or Reminder Any subclass that overrides 'eql?' should also override 'hash' appropriately. ## Long version Probably all of us will agree that equality is very important... but it will vary depending on what we are trying to compare. In ruby we have different methods to compare objects... (==, ===, eql?, equal?), and it is very important to know their differences. The '==' is the 'daily used one' and in descendant subclasses. https://ruby-doc.org/core-2.5.3/BasicObject.html#method-i-3D-3D As described in the documentation subclasses, For objects of class Object, 'eql?' is synonymous with '==' and subclasses normally continue this tradition by aliasing 'eql?' to the overriden '==' method. We have done this in different places in YaST (just look for 'alias_method :eql?, :=='). But in most of the cases we have omitted the 'hash' method override which could be a problem in case we use Hashes or Arrays. And this is basically what has happened in a recent bug where the user had 16 interfaces configured and adding one more deleted all his routes. (What? 16 is fine but 17 is not?) https://ruby-doc.org/core-2.7.0/Array.html#method-i-2D https://github.com/ruby/ruby/blob/master/array.c Basically the 'difference' or '-' methods use 'eql?' for comparing elements when the array is smaller than 17 elements and the␣ 'hash' when it is bigger. Some Links: - Recent bug https://bugzilla.suse.com/show_bug.cgi?id=1186082 - https://github.com/yast/yast-storage-ng/pull/491/commits/1144a6ee3d88154f92f... - https://stevenharman.net/well-behaved-ruby-object-equality -- Knut Alejandro Anderssen González YaST Team at SUSE Linux GmbH
El viernes, 21 de mayo de 2021 14:55:24 (WEST) Knut Alejandro Anderssen González escribió:
## Short version or Reminder
Any subclass that overrides 'eql?' should also override 'hash' appropriately.
Hi Knut, Thanks a lot for debugging and figuring out which was the problem! Regards, Imo -- Imobach González Sosa YaST Team at SUSE LLC https://imobachgs.github.io/
On 5/21/21 2:55 PM, Knut Alejandro Anderssen González wrote:
## Short version or Reminder
Any subclass that overrides 'eql?' should also override 'hash' appropriately.
By default, the methods #==, #eql? and #equal? in Object subclasses return true if both objects are the same (point to the same object in memory). Well, actually #eql? should return true if both objects refer to the same hash key. But in practice, two objects have the same hash key if they are the very same object. There are some exceptions like String, which returns the same hash key if they have the same value. The #eql? and #hash methods must be related. That is, if two objects are #eql?, then they should have the same #hash. This is important, otherwise we could have unexpected results in certain operations like subtracting Arrays. When performing the difference of two Arrays, the method used for comparing the objects in the Array depends on the Array length (see source code of Array#difference [1]). If both Arrays have more than SMALL_ARRAY_LEN (i.e., 16) elements, then the #hash method is used. Otherwise it uses #eql?. This is one of the reason why #eql? and #hash should be paired. In YaST code, we tend to overload #== and then to make #eql? an alias to the overloaded #==. This is dangerous if we forget to modify #hash properly. For example, the result of Array<OurClass> - Array<OurClass> would depend on the length of the arrays. To avoid these problems, I think we should keep the same logic for #eql? and #hash. And optionally, we could do #== an alias to #eql?. I have prepared a proposal in yast-users [2]. It consists on a mixing to implement the "Equatable" property. What do you think? If you agree with this solution (or something) similar, then we could move it to a more generic place like yast2 repo. Regards, Iván [1] https://ruby-doc.org/core-2.7.0/Array.html#method-i-difference [2] https://github.com/yast/yast-users/pull/283/files -- José Iván López González YaST Team at SUSE LINUX GmbH IRC: jilopez
participants (3)
-
Imobach Gonzalez Sosa -
José Iván López González -
Knut Alejandro Anderssen González