4 ms·
"On the surface, this looks reasonable." No, it actually doesn't. class Item def add_to_cart(cart) cart.increase_total(this.price) This is where
by yxhuvud 11y ago
"On the surface, this looks reasonable."
No, it actually doesn't.
class Item
def add_to_cart(cart)
cart.increase_total(this.price)
This is where the trouble is. It is not the inheritance, it is the add_to_cart method not being in the correct class. If this instead had been
class Cart
def add(item)
.. add item to some collection of stuffs implementing price().
def sum()
.. calculate total sum.
then the whole problem evaporates. If your code reads do_something_to_foo(foo), then very often usually it really ought to be foo.do_something()
Inheritance can be ok when used sparingly, but this example is not one of those.
- cubano 11y agoYes totally agree. The best way that I learned OO was when it was explained that, in most cases, its best to take objects as nouns (ie Item, Book, Prices etc) and the object methods should be verbs (ie do_something() or act_on_this() etc)
- thomasahle 11y ago'item.add_to(cart)' seems to work fine from a gramatical point of view. It's only when we think in terms of ownership "The cart should own the item" that we choose the right order, no?
- wvenable 11y agocart.add(item) is also grammatical and less conceptually complex than any sort of "add_to" construct.
- enraged_camel 11y agoYep. There is no reason the Item class should "know about" the Cart class. The add_to_cart method doesn't belong in it.
- lbarrow 11y agoI concede the example is a bit of a straw man. But I'd challenge you to provide a similarly concise problem and solution where 1) inheritance is useful and 2) there isn't a better solution that doesn't use inheritance.
- sytelus 11y agoThe classic examples... Person > Employee > Manager Shape > Rectangle, Circle Object > anything Product > Book, TV I think these are good examples where inheritance makes lot of sense. There is commonality between things and there is specialization (overrides) or definitions (abstract methods). It's wrong to say that the concept of inheritance is evil. Like pretty much everything, it can be severely abused (very deep hierarchies, multiple on inheritence etc). The classic example of OOP abuse is probably things like MFC library.
- thomasahle 11y agoWould you also support Shape > Rectangle > Square? Or are all your super classes abstract?
- Bluestrike2 11y agoIf we're being cheeky, a square is also a rectangle. Unless I'm misreading your post, I'd expect your superclasses to be as abstract as they needs to be and no more: Vehicle < Car, Truck, etc. Person < Employee, Customer, etc. There's a pretty obvious logic to them that doesn't really demand much extra thought. I can't think of anyone outside of an irreverent ontologist who would suggest going beyond that unless there's a valid reason for it, like say: PhilosophicalBeing < Person < Employee, Customer, etc.
- tamana 11y agoA mutable square is not a mutable rectangle, because a mutablr rectangle can change its width independently of its length. This is the classic problem of contravariance: subtyping in the face of mutability.
- contravariant 11y ago>If your code reads do_something_to_foo(foo), then very often usually it really ought to be foo.do_something() Personally I'd make an exception when using extension methods in C#. If you make an extension method which looks (schematically) as follows: add_to_cart(this item, Cart cart) { cart.add(item); return item; } You can write code that looks like: item.add_to_cart(cart) .check_availability() .do_something_else() .etc() Well, this example isn't the best, but you get the general idea.
- parenthephobia 11y agoYou're hand-waving a lot away with "some collection of stuffs implementing price()". That stuff was the actual problem the article was about. The question of how it gets in the cart and how and when the total is calculated is irrelevant. The actual problem of the article was that if the stuff you put in a cart simply derives from a single base class, then any new stuff-based behaviour (potentially) requires changing all subclasses. For example, if you wanted to to add shelf life to perishable items and have the cart maintain the minimum shelf life, then you have to add shelf life to the item class and potentially override it on all subclasses. An alternative implementation would be to define a Perishable interface with a ShelfLife method and only implement that for perishable items. class Cart def minimumShelfLife items.ofType<Perishable>().minimum(item -> item.shelfLife, ifEmpty: infinity) This avoids cluttering up the base class with loads of methods which won't be meaningful for most of the types, and which may cause problems when extending things later. I'm not convinced, but I believe that's the essence of the argument the article intends to make.
- wvenable 11y agoBut that's not true as it's not a binary choice. The idea of adding a Perishable interface works just as well whether or not items extend a single base class. Sometimes cluttering the base class is a better solution if there is a meaningful default. For example, if you start with all non-taxable items and the requirements change for taxable items -- it makes sense to implement in the base class as a Tax property that defaults to zero. Then you don't need conditionals and interface checks whenever you want to display or do tax calculations.
- mediaobelia 11y ago> This avoids cluttering up the base class with loads of methods which won't be meaningful for most of the types I'd argue that shelfLife is meaningful for all Items, and it's unnecessarily presumptuous to assume only Carts will want to know about shelfLife. Plus it's less code to simply add a shelfLife method in Item to return infinity. Later, when you realize expirationDate is what you really want to know you'll have less code to modify to get the logic correct. In most cases you'll have fewer problems doing things the traditional OO way and sending the expirationDate message directly to the item.