Documentation: Virtus-1.0.0 upgrade breaks if a class does self.inherited without super()

[tomoyoirl]

We have a class like so:

class Parent
  include Virtus
  attribute :foo
  def self.inherited(subclass)
     blahblah(subclass)
  end
end

class Child < Parent
end

We upgraded to 1.0.0, replaced include Virtus with include Virtus.model. When our code attempted to run Child.new it got an error:

NoMethodError: undefined method set' for NilClass`

After investigation, it became obvious that this was because our self.inherited did not super and so the class's attribute set was not initialized. Adding super was not a problem, but the need for this change was not documented anywhere. :)

[elskwid]

@twhaples, thanks for pointing this out. I have a long-standing issue to polish up the docs. I will make sure we get a note about that in there.

[solnic]

Even though I understand it might've been a "gotcha" for you, it's usually, well, it's always a good idea to call super if you override something :smiley: Anyway, I agree this should be mentioned in the docs for the sanity sake :smile:

[dkubb]

An even more generalized rule is called the Liskov Substitution Principle. The formal definition is a bit convoluted, but it basically means that when you subclass something, any methods you override should not change the desirable properties of the overridden method. You should be able to substitute any subclass instance for parent class instances and not change the important properties of the system.

If you find yourself wanting to subclass and change important behaviour, you're better off using composition instead of inheritance.

In this specific case the inherited hook in an ancestor does something important. By choosing not to call super the behaviour is changed.

[elskwid]

Ooh, look at @dkubb calling out the LSP for all to see!

Seriously though, thanks for that point, it's a great one to keep in mind.