Derived classes of ValueObject that define attributes have have an incorrect == method

[purpleD17]

Setting this up in irb:

require 'virtus'

class Teacher
  include Virtus::ValueObject
  attribute :id, String
end

class TeachingAssistant < Teacher
  attribute :major, String       
end     

and executing

TeachingAssistant.new(id: 1) == TeachingAssistant.new(id: 2)

returns true. Note if you switch the person object to a normal Virtus object like so:

class Teacher
  include Virtus
  attribute :id, String
end

The comparison returns false as expected

[elskwid]

@purpleD17 - I'm poking around at this now and noticed something. If I define the same classes and then create two instances ta1 and ta2 (from your example above):

ta1.class.equalizer.instance_variable_get "@keys"
# => [:major]

ta2.class.equalizer.instance_variable_get "@keys"
# => [:major]

It looks like Equalizer isn't using the originally defined keys on the derived classes. I'll update as I find more.

[elskwid]

You can see it further here:

ta1.inspect
# => "#<TeachingAssistant major=nil>"

ta2.inspect
# => "#<TeachingAssistant major=nil>"

t = Teacher.new
# => #<Teacher id=nil>

[elskwid]

And, right here keys are only added to the equalizer when attribute is called. It doesn't look like there is any handling when a class inherits from a Virtus::ValueObject.

[elskwid]

Here's an idea of how it could be fixed (in my example I'm creating a new equalizer method just to test the idea). In the end, we could copy the existing attribute_set do equalizer when it is first instantiated. This should catch the attributes that are copied from the parent class.

https://gist.github.com/elskwid/6505679

[solnic]

We should keep that in mind when porting ValueObject to the extracted Equalizer. Adding this issue to 1.0.0 milestone

[elskwid]

@purpleD17 - do you have an urgent need for a fix or a workaround? @solnic is busy doing some major refactoring but perhaps we could figure something out if you need it.

[craiglittle]

Just wanted to add myself to the list of people affected by this. In my case, I can include Virtus in lieu of Virtus::ValueObject and forgo the niceties of ValueObject for the time being, but I'd really like to see a fix. :)

[solnic]

It's gonna be fixed in 1.0.0. Just need to finish axiom-types-integration first.

[craiglittle]

No worries, thanks for your work!

[purpleD17]

Hi, thanks for looking at this. @elskwid, We got around this temporarily when we discovered the problem by doing this (within our own module for namespacing):

class ValueObject
  include Virtus::ValueObject

  # overwrite `==` for each ValueObject subclass since it seems Virtus
  # has a buggy implementation of `==` with inheritance.
  # Note: Virtus will overwrite `==` each time a subclass defines a new
  # attribute (eg. `attribute :foo, Foo`), so we have to overwrite that
  def self.inherited(base)
    base.send(:define_method, :==) do |other|
      attributes == other.attributes
    end
    super
  end
end

We can git rid of this once you land the fix.

[solnic]

btw it's fixed in axiom-types-integration branch