Get Rid of That Code Smell – Primitive Obsession

There's another code smell that every programming whiz kid produces at some point: over-engineered.<p>All code has a cost, primitives have a lower base cost because they are universal in the language and thus every programmer will automatically know how to use them. Before introducing a custom object with yet another API to be learned, you need justification.<p>&#62; <i>Implement Money class if you need to deal with money, it’s much better than using floats all over the place. Don’t use Hash for configuration objects, use a custom Configuration class instead.</i><p>Let's pretend he said integer since floats for money is outright broken. Yes, Money is a clearcut case where having a money-specific API is both intuitive and immediately useful.<p>Configuration on the other hand is debatable. If you're just doing things a hash does then it's fine. If you find yourself with lots of helper methods or repeating the same transformations over and over then that's when you have a code smell that may warrant lifting configuration to its own level.<p>But the critical thing is to remember that there's a non-trivial cost to creating domain-specific objects.

This is <i>exactly</i> the opposite of the advice recently given by Rich Hickey in his keynote at RailsConf 2012, where he strongly recommends using simple, transparent data structures without a lot of OO wrapping baggage.<p>Personally I'm on the fence but when you look at how much boilerplate this Virtus example needs to get rid of the code "smell" you have to wonder if maybe he has a point.

Do not hastily judge sticking to primitives as bad. Here's a relevant SICP quote:<p><i>"The simple structure and natural applicability of lists are reflected in functions that are amazingly nonidiosyncratic. In Pascal the plethora of declarable data structures induces a specialization within functions that inhibits and penalizes casual cooperation. It is better to have 100 functions operate on one data structure than to have 10 functions operate on 10 data structures."</i>

The potential danger in computationally intensive code is that you have a number of wrappers around the actual data (e.g. in the article AttributeSet wraps a hash table), each requiring a pointer dereference and a vtable lookup.<p>So, IMO it depends on the language features available and the scenario whether it is a code smell. For instance, Haskell allows you to introduce a new type whose data representation is the same as the original type using <i>newtype</i> [1]. In contrast to a type alias, <i>newtype</i> makes a different type, but it still allows you to define functions on that type in terms of the original type (using the type's constructor). If you want to hide the underlying representation, you simply do not export the constructors from the enclosing module.<p>tl;dr: in Haskell you do get a better abstraction without the overhead. I am pretty sure many other language can do as well (private inheritance in C++?).<p>[1] <a href="http://www.haskell.org/onlinereport/decls.html#datatype-renaming" rel="nofollow">http://www.haskell.org/onlinereport/decls.html#datatype-rena...</a>

It's an interesting article, but I'm not really sure this is the best approach. In terms of having the 'best' code that is 100% OO then sure, but in terms of actually having usable, understandable, code I'm not sure I agree.<p>If you return a Hash of attributes everyone knows how a Hash works, but if you return a custom Attributes object people have to learn how it works, there could be bugs, etc.

I understand about making code more clear, but I'm not sure adding complexity/boilerplate solves that in every case- the even more extreme case would be to do away with the class entirely and just have a hash type property that gets passed around.... his very verbose counter-argument is the other extreme, and while I see his point, I tend to want to start with the property and only move towards the "rich object" when I'm sure that it's the best solution....

just make it work, as simply as possible.