Open twhaples opened 10 years ago
@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.
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:
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.
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.
We have a class like so:
We upgraded to 1.0.0, replaced
include Virtus
withinclude Virtus.model
. When our code attempted to runChild.new
it got an error:NoMethodError: undefined method
set' for NilClass`After investigation, it became obvious that this was because our
self.inherited
did notsuper
and so the class's attribute set was not initialized. Addingsuper
was not a problem, but the need for this change was not documented anywhere. :)