Is polymorphism YAGNI or OCP?

Discussion in 'Web Design and Development' started by Jamesbot, Jul 31, 2014.

  1. Jamesbot macrumors member

    Joined:
    Jun 19, 2009
    #1
    Say a user can add a product to an order.

    Code:
    class Order < ActiveRecord::Base
      has_many :line_items
    end
    
    class LineItem < ActiveRecord::Base
      attr_accessible :price, :quantity
    
      belongs_to :order
      belongs_to :product
    end
    
    class Product < ActiveRecord::Base
      attr_accessible :price
      has_many :line_items
    end
    
    Now I have no idea if in the future they're gonna want to add things other than "products" into a order. I can image something like this could happen:

    Code:
    class PowerFeature < ActiveRecord::Base
      attr_accessible :price
      has_many :line_items
    end
    
    So to make this work, I'd make LineItem polymorphic:

    Code:
    class LineItem < ActiveRecord::Base
      attr_accessible :price, :quantity
    
      belongs_to :purchasable, polymophic: true
    end
    
    class Product < ActiveRecord::Base
      attr_accessible :price
      has_many :line_items, as: :purchasable
    end
    
    class PowerFeature < ActiveRecord::Base
      attr_accessible :price
      has_many :line_items, as: :purchasable
    end
    
    So is making LineItem polymorphic from the start YAGNI or is not doing so violating Open/Closed (since I'll need to alter LineItem as soon as they want to add something to an order that's not a Product)?

    Thx!
     
  2. chrono1081 macrumors 604

    chrono1081

    Joined:
    Jan 26, 2008
    Location:
    Isla Nublar
    #2
    In my seven years of computer science classes and my 16 years of programming I've never came across the terms YAGNI and OCP...I'm pretty sure due to the lack of responses others haven't either...
     
  3. Jamesbot thread starter macrumors member

    Joined:
    Jun 19, 2009
    #3
    Fair enough.

    YAGNI stands for you aren't gonna need it. Jeff Atwood wrote a pretty good blog post about it here.

    Here's a relevant quote:

    So I've been burned by this before. When we plan features at my job, I usually sit in a room with various stakeholders and we come up with an initial feature plan. When this happens there are lots of ideas thrown around. I then go off and start coding a prototype. Usually at this time I'm pretty uncertain which direction the feature will go, so I try to code something flexible while satisfying the original specs.

    Sometimes they'll come back a month later and say "this feature works great, can we do basically the same thing but with this other thing and a little bit differently?" in which case I'm happy I built in the flexibility.

    Other times, they'll say "well it turns out we don't actually need all the things we originally asked for, can you remove those features?", and I'm left with a design that's overly complex for the amount of functionality it provides. In which case I not only wasted time building something I didn't end up needing, but I have to spend additional time to go back and refactor to something simpler.

    So YAGNI's like "just don't do it". You can't predict the future, so just build the simplest thing that works.

    On the other hand, we have SOLID which is a set of design principals for building stable, flexible object-oriented software. One of those principals is called the Open/closed principle or OCP.

    Here's a quote from the wikipedia article:

    I've been burned by this before as well. Say I have a class called "Attachment" and it represents an uploaded file. Originally, it was spec'd that a user can upload an attachment to a document. A month later they're like "attachments are great! Let have attachments for Quotes as well!" So the guy coding it does something like this:

    Code:
    class Attachment < ActiveRecord::Base
      attr_accessible :document_id, :quote_id
    
      def quote
        return unless quote_id
        Quote.find(quote_id)
      end
    
      def quote_attachment?
        return true if quote_id
      end
    
      def document
        return unless document_id
        Document.find(document_id)
      end
    
      def document_attachment?
        return true if document_id
      end
    end
    
    Months go by and all kinds of thing end up getting attachments, and every time they add a new one they follow the same pattern of adding pairs of methods for every type of "attachable". Worse you end up seeing stuff like this all over the place:

    Code:
    if attachment.document_attachment?
      render attachment.document
    elsif attachment.quote_attachment?
      render attachment.quote
    else
      render text: "Couldn't find!"
    end
    
    So this kind of thing spreads around, and for a lot of developers its easy to just not think about it and just copy and paste in another conditional and change it slightly for the new type of thing.

    So my attachment class is making a mess because its violating both OCP and tell don't ask. And I'm wishing that someone had just solved this problem from the start instead off letting this thing grow into a mess of copy/paste code.

    So my question: is doing something like replace conditional with polymorphism early on YAGNI, or should I generally wait unless 2nd or 3rd extension to this class, knowing that it could potentially end up growing to be a mess if other developers are tempted to continue extending via cut/paste.

    Hope that makes sense! I know there are some experienced developers reading this forum and that while the answer is probably "it depends on the situation", some insight gained from said experience is certainly welcome. I haven't been programming professionally for very long and I'm finding making judgement calls in uncertain situations challenging.
     
  4. mrichmon macrumors 6502a

    Joined:
    Jun 17, 2003
    #4
    When that happens it is time to refactor.

    In any moderately complex software your understanding of the problem domain changes over the development life of the code. This almost always means that some of your early assumptions are invalidated as time goes on.

    The primary reference for refactoring is: http://martinfowler.com/books/refactoring.html
     
  5. Jamesbot thread starter macrumors member

    Joined:
    Jun 19, 2009
    #5
    Yeah I read that book (well, actually I read the ruby version of that book, also written by fowler)..

    He says "The first time you do something, you just do it. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway. The third time you do something similar, you refactor".

    I guess I'm just having an issue with the "wait 3 times" thing. If the duplication is pervasive enough and I'm doing shotgun surgery, I'm like "man, should I really wait until the third time this happens before I refactor?
     
  6. mrichmon, Aug 11, 2014
    Last edited: Aug 11, 2014

    mrichmon macrumors 6502a

    Joined:
    Jun 17, 2003
    #6
    It sounds like you are really asking about judgement calls. Ultimately it comes down to:

    If the existing codebase/development style is getting in the way of the next development iteration then it is time to refactor or re-think the existing approach.

    On some projects you can go a few years before the codebase starts to get in the way. On other projects, it might be a couple of weeks or months.

    There are too many variables, both qualitative and quantitative, that preclude the possibility of a "do X when Y" type rule.
     
  7. Jamesbot thread starter macrumors member

    Joined:
    Jun 19, 2009
    #7
    Yea that sounds reasonable. I guess the verdict is to not let uncertainty dictate design choices, but to really do the simplest thing possible in the moment. Unless changing the code for new requirements is too painful right now this second.

    For what its worth I've never worked on a project that lasts for years, and right now it feels like requirements are changing every couple of weeks.

    Anyway, thanks for the help.
     
  8. iSee macrumors 68040

    iSee

    Joined:
    Oct 25, 2004
    #8
    I generally agree these kinds of things are judgement calls.

    But in these specific examples, I would generally model the relationships between objects/entities/things so that the child/owned/contained thing doesn't know about the parent/owner/container but the parent/owner/container knows about the child/etc. That is, Product and PowerFeature know about LineItems but LineItems don't know about Products or PowerFeatures (or any future thing that may be composed of LineItems). Likewise, Attachments wouldn't know about the kinds of thing that can contain them.

    (And sorry if I'm misunderstanding the example... I'm not familiar with the syntax, so I could be missing something.)

    Now, I know relationships are pretty typically expressed as you have them in the relational systems RDBMSs use. But that's inverted from (what I think of) as is natural for an OO design that seem preferable at application runtime.

    Ideally, an ORM would take the pain out of dealing with this "impedance mismatch" so that, e.g. in your runtime object model, Product and PowerFeature simply each have a list of LineItems, while your LineItem table may have both a product_id and powerfeature_id (or maybe there's a ProductLineItem table and a PowerFeatureLineItem table, or whatever.) and the ORM layer would make this transparent to the application.
     
  9. Jamesbot thread starter macrumors member

    Joined:
    Jun 19, 2009
    #9
    I didn't mention this in my first example, but imagine that a user is looking at an order they placed. We want to be able to show them a list of all the things they ordered. Something like this:

    Code:
    # Order Summary
    
    - Ship to: Tom Hanks
    - Via: FedEx Air
    
    ## Line Items
    
      1. Wilson Sporting Goods Volleyball
          diameter: 27"
          color: white
          price: $11.40
    
      2. Ice Skate
          size: 10.5
          material: leather
          price: $41.99
    
      3. Portable Toilet
          capacity: 60 Gallons
          price: $672.00
    
    Total Amount: 725.39
    
    So I wanna be able to say something like: for this Order, get all its LineItems, and for each LineItem, tell its "Purchasable" to render itself.

    In this context, it makes sense that the LineItem knows what its purchasable is.

    I'm not sure how you would do it the other way around. Maybe you can provide an example.

    So this is exactly the kind of thing I'm trying to avoid. Cause now every time we add a new kind of purchasable, I'm going to have to either:

    A. Create a new column on my line_items table, and write some code to determine what kind of purchasable it is (and update this code everything I add a new type).

    or:

    B. Create a new model (PowerFeatureLineItem), create a new table (power_feature_line_items), most likely a superclass (LineItem) and somewhere a client's gonna be asking which kind of LineItem this is (as a case, or if/elsif/else).

    So in ActiveRecord, we have a feature where we can make a relationship "polymorphic" by adding 2 columns: "purchasable_id" and "purchasable_type". Here "purchasable_type" is storing the class name of the "purchasable".

    For example:

    Code:
    
    class PortableToilet
      belongs_to :line_item, as: :purchasable
    end
    
    class LineItem < ActiveRecord::Base
      belongs_to :purchasable, polymorphic: true
    end
    
    line_item = LineItem.first
    
    # LineItem Load (0.6ms)  SELECT "line_items".* FROM "line_items" ORDER BY "line_items"."id" ASC LIMIT 1
     => #<LineItem id: 1, purchasable_id: 37, purchasable_type: "PortableToilet", order_id: 1>
    
    purchasable = line_item.purchasable
    
    # PortableToilet Load (0.7ms)  SELECT "portable_toilets".* FROM "portable_toilets" WHERE "portable_toilets"."id" = 37 LIMIT 1
    => #<PortableToilet id: 37,  capacity: "60 gallons", price: "672.00">
    
    purchasable.go_render_yourself!
    => "... display some markup..."
    
    
    Here when you ask a LineItem for its purchasable, ActiveRecord looks up the LineItem's purchasable_type and uses that to determine which table to query. And as long as all the different kinds of purchasables respond to "go_render_yourself!", you can print a representation of each purchasable with out having to use a long case statement querying on type in your view code (or wherever else).

    Anyway, the question was more about how early is too early to design for ease of extension. In this example I could have built it in a really simple way, but shortly after someone else could be working on this code and decide to use an if statement querying on type code, which later on turns into a case statement querying on type code, witch then introduces inheritance to handle the duplication between types, which turns into "hot damn this thing is both complex and hard to change."

    I think this happens because these kinds of decisions are hard and its easier wrap your head around if statements (in the beginning), so you just throw one in there and move on, and from then on everyone just follows the pattern instead of adding a level of abstraction (and taking the time to refactor, which is not always easy to do).
     
  10. iSee macrumors 68040

    iSee

    Joined:
    Oct 25, 2004
    #10
    (Sorry, I did not respond sooner, there was no quote notification.

    Oh, OK, I think I understand.

    ActiveRecord is an ORM.

    I think a big part of the issue here is that we're talking about both a relational model and an OO model for your domian at the same time. That's fine. You're working with an ORM, after all.

    But SOLID is an object-oriented design thing. A natural model typically willnot map cleanly to an OO model and you can't necessarily apply OOD concepts to a relational model. (That's why ORMs are even a thing in the first place. If you simply had object stores -- or rather, object-graph stores -- there'd be no component there in your architecture. Don't get me started! :) )

    I know nothing about ActiveRecord, but the part you are showning here exposes a relational model.

    So, on your original question I'd say: don't expect to be able to apply SOLID principals to the relational parts of your system. YAGNI, on the other hand, is applicable across the board. So I'd go with YAGNI in this case.

    To be clear what I mean by relational and OO models, here's a simplified document, email, attachement model in both relational and OO forms (in basic UML notation):

    OO Model

    Document
    -----------
    filename: string
    attachements[*]: Attachment

    Email
    ------
    subject: string
    ...
    attachments[*]: Attachement

    Attachment
    -------------
    filename: string
    content: binary

    That is, a Document has a collection of zero or more Attachments. So does Email. Attachment has no knowledge of what might have attachements so free and easy to make a third kind of thing that has a collection of attachments or just one attachment or whatever.

    Relational

    Document
    -----------
    id: ID
    filename: string

    Email
    ------
    id: ID
    subject: string
    ...

    Attachment
    -------------
    id: ID
    document_id: ID <nullable>
    email_id: ID <nullable>
    filename: string
    content: binary

    (Here, the non-null document or email id is the container for the attachment. Alternatively there could be two tables or a single container id and a type. To me, a good ORM will let the application work in terms of the OO model and will let you choose (and easily change) the relational model based on performance or possibly some partitioning scheme.)
     

Share This Page