Is polymorphism YAGNI or OCP?

Jamesbot · Jul 31, 2014

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!

chrono1081 · Aug 8, 2014

Jamesbot said:
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!

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...

Jamesbot · Aug 8, 2014

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:

As developers, I think we also tend to be far too optimistic in assessing the generality of our own solutions, and thus we end up building elaborate OOP frameworks around things that may not justify that level of complexity. To combat this urge, I suggest following the YAGNI (You Aren't Gonna Need It) doctrine. Build what you need as you need it, aggressively refactoring as you go along; don't spend a lot of time planning for grandiose, unknown future scenarios. Good software can evolve into what it will ultimately become.

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:

Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"; that is, such an entity can allow its behavior to be modified without altering its source code.

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.

mrichmon · Aug 11, 2014

Jamesbot said:
... knowing that it could potentially end up growing to be a mess if other developers are tempted to continue extending via cut/paste.

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

Jamesbot · Aug 11, 2014

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?

mrichmon · Aug 11, 2014

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.

Jamesbot · Aug 11, 2014

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.

iSee · Aug 12, 2014

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.

Jamesbot · Aug 13, 2014

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.

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.

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.)

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).

iSee · Aug 19, 2014

Jamesbot said:
...

(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.)

Search

Search

Is polymorphism YAGNI or OCP?

Jamesbot

macrumors member

chrono1081

macrumors G3

Jamesbot

macrumors member

mrichmon

macrumors 6502a

Jamesbot

macrumors member

mrichmon

macrumors 6502a

Jamesbot

macrumors member

iSee

macrumors 68040

Jamesbot

macrumors member

iSee

macrumors 68040

Our Staff