
Object Oriented Design principles Java programmers should know - javinpaul
http://javarevisited.blogspot.com/2012/03/10-object-oriented-design-principles.html
======
tinco
The author lacks conviction:

    
    
        "if you use a hardcoded value more than one time consider making it public final constant"
    

This should obviously be: "if you use a hardcoded value, make it a public
final constant"

And:

    
    
        "if you have block of code in more than two place consider making it a separate method."
    

This should be: "if you have a block of code, consider making it a separate
method"

Also, half of these principles imply the other ones.

edit: This is meant seriously btw, if you're giving advice to novice
programmers, the instructions should be clear. If you say 'consider making a
separate method' what should the programmer consider? If the programmer is new
to this he or she won't know what considerations to make either.

This goes for all principles by the way, they're all fine and dandy but
they're also trade offs. Every time you abstract something, you risk
obfuscating the code for the uninitiated.

As arethuza suggests, Keep It Simple Stupid is also a solid design principle,
and it contradicts a whole lot of the other principles.

~~~
keyle
I don't agree with "if you have a block of code, consider making it a separate
method". I've seen it done and done it myself in the past, and it ended up
being a whole lot of methods used only once and described well (by name).
However it added unnecessary complexity which added to the cost of ownership
of the classes. I hope someone can relate.

~~~
konradb
I can relate - it can also be a way where you can introduce meaning to your
code. As opposed to a a series of instructions without clear intention, you
can put them in a function that has a name that signals your intention. So you
are labelling what that code does, using language features. I'm sure there are
plenty of examples of this done well and done badly.

~~~
eru
Not only meaning, but also separation. Two chunks of code after each other in
a method can communicate via all kinds of mechanisms, e.g. having the same
variable in scope. As a reader you will have to check for these communications
to understand the method, even if those two blocks of code do not in fact
communicate.

If you separate the block out into their own methods, they reader will have a
much easier time verifying that they are decoupled.

------
wheaties
Some of us working in hybrid OO-FP languages or strictly FP languages might
argue against IoC. I have seen and been exposed to code bases while
beautifully written are completely untraceable in cause-effect debugging.
Which combination causes the error? Which config was that environment set to?
Let me try to divine the structure...

~~~
narag
I've always worked with OO, not FP. I dislike IoC for the same reasons. Also
it's anti-KISS and debilitates encapsulation.

The worst effect of this kind of doing things is the image of programming that
it shows to beginners. Programming well has nothing to do with that, really.

------
taeric
There is one other "principle" that I think all programmers should keep in
mind: Programs typically exist to satisfy a user's need, not provide some
intrinsic code value to you.

~~~
meaty
Which has absolutely nothing to do with OO specifically.

~~~
taeric
Agreed. But all too often I see people trying to adhere to some paradigms
"principles," at the expense of the user value.

I mean, of course you shouldn't try and deliberately flaunt any good
guidelines, but the measure of your code is ultimately not these.

~~~
meaty
I think that is implicit.

The guidelines set out here allow you to deliver that value without having to
rip up your product for every minor change.

~~~
w0utert
The problem I have with lists of 'principles' like this, is that they usually
carry the presumption that there are no exceptions to any one of them. IMO
this is not a good way to introduce novice programmers to sound software
architecture, and often leads to an unmaintainable mess of 'flexible,
extensible, loosely-coupled code' that does not 'repeat itself' and
'encapsulates and abstracts away implementation details'. Especially in
situations where a) none of these things actually matter, or b) most (or all)
of them could have been realized with much less complexity.

Just as an example, I really hate principles like 'program to interface, not
implementation'. More often than not, you are not writing 'extensible
frameworks' or 'reusable classes', so hiding each.and.every.class behind an
interface serves no purpose. I get really depressed browsing the typical Java
or C# code written by all the architecture astronauts around me that are
obeying some variant of the '10 object oriented design principles' and
thinking they need to make ISomething for every Something constructed by a
AbstractSomethingFactory they can dream of, even though it's completely
obvious only one kind Something will ever exist.

TL;DR: there's almost as much risk creating crappy code blindly following what
is commonly sold as 'OO design principles' as there is in hacking up
everything with complete disregard of sound design and architecture.

I think this should come pretty close to what your parent poster was trying to
say. I also think many developers working in languages that aren't strictly OO
would agree.

~~~
meaty
It's not about how many ISomething's exist - it's about defining a contract
between Something and the rest of the system.

When it comes to testing, how are you going to test your ClassX if it's
tightly coupled to ClassY rather than IClassY (which can be mocked/stubbed)?
Oh, you have to test both.

Also, an interface is a great way of implementing "worry about that shit later
- just let me talk to it now". You can define and evolve the contracts before
you have to refactor expensive implementations.

As for AbstractSomethingFactory, I've not written a bit of code like that for
night on 10 years. The container gives me what shit I need, ready configured
with all dependencies.

Blindly following rules is dangerous, but burning entire books for what
appears to be lack of experience is much more dangerous.

~~~
w0utert
>> _It's not about how many ISomething's exist - it's about defining a
contract between Something and the rest of the system._

Why would I need to introduce a separate interface class to define this
contract, instead of making it explicit through the public class interface?
What problem do I solve by adding another layer of abstraction between
Something (the class) and Something (the interface), if Something is just some
internally used entity that isn't part of a framework or public API? I've seen
millions of lines of code to date, and the number of times hiding public class
interfaces behind interface classes (note how those sound almost identical,
wonder why that would be?) can be counted on the fingers of one hand.
Interface classes only make sense if you expect classes to be used like you
would use components in a component-based architecture, e.g. for plugins, or
polymorphic, adaptable structures that may be implemented by combinations of
classes, etc.

>> _When it comes to testing, how are you going to test your ClassX if it's
tightly coupled to ClassY rather than IClassY (which can be mocked/stubbed)?
Oh, you have to test both._

You don't always need interface classes for loose coupling, abstract base
classes cover many cases, and in many other cases loose coupling is simply not
required. There is nothing wrong in testing multiple classes that are tightly
coupled together, without stubbing. In those cases where loose coupling is
absolutely essential (frameworks for example) and you want to compose classes
to provide some kind of interface, use interface classes. My point is not that
interfaces classes are wrong, but they are overused, and more often than not,
they add complexity without benefits.

>> _Also, an interface is a great way of implementing "worry about that shit
later - just let me talk to it now". You can define and evolve the contracts
before you have to refactor expensive implementations._

Again, I don't see why you would need interface classes for any of this. I can
write a stub class that let's me 'worry about shit later' simply by defining
its public interface and leaving its implementation empty. If I have a lot of
classes adhering to the same (semi-) abstract interface and I don't want to
stub them all with empty methods, I simply create a temporary base class I can
remove later. Eventually I'll have to implement everything anyway, and I'm not
writing interfaces just for the fun of it, or the remote possibility that some
interface may be required later, even though no-one asked for it.

>> _As for AbstractSomethingFactory, I've not written a bit of code like that
for night on 10 years. The container gives me what shit I need, ready
configured with all dependencies._

Interesting that you should say that, because I used AbstractSomethingFactory
simply as a caricature of an often overused design pattern, without implying
that you should never write an abstract factory. In fact, abstract factories
are a useful solution in many common designs, even though they are so
overused.

>> _Blindly following rules is dangerous, but burning entire books for what
appears to be lack of experience is much more dangerous._

Which means we have to aim somewhere in between, which is exactly what I was
trying to say in the first place. Unfortunately many developers somehow seem
to drift to either one of the extremes, the Java/C#-like language programmers
to the 'create moar classes!' side, everyone else to the 'just make it work'
side. My point is that both lead to unmaintainable, bug-ridden code.

~~~
meaty
Firstly the interface isn't a layer of abstraction - it's a contract, a
boundary. An abstraction is an arbitrary concept which may or may not apply to
an interface.

Secondly, the abstract base concept is actually a deadly form of coupling. As
many people have suggested since the dawn of the problem, composition is
better than inheritance and what sits at the composition boundaries?
Interfaces! Inheritance usually turns into an LSP-violating [1] clusterfuck. I
know - I have spent 2 weeks refactoring one into something which doesn't stick
a fork in your eye every time you change something. And this is a non trivial
one with over 100 classes in the inheritance graph (this is a roughly vomited
out version of the Party archetype from the MDA book [2]).

So your tests are chock full of empty stub classes rather than mocks? Yuck.

If you look at ASP.Net's native API (before System.Web.Abstractions hid all
the crimes away), you will see the sort of shit you'll get yourself into if
you follow your own advice.

Agree with your last point though.

For reference, yes I am an architecture astronaut with one foot nailed to the
ground :)

[1] <http://en.wikipedia.org/wiki/Liskov_substitution_principle>

[2] [http://www.amazon.com/Enterprise-Patterns-MDA-Building-
Arche...](http://www.amazon.com/Enterprise-Patterns-MDA-Building-
Archetype/dp/032111230X)

~~~
w0utert
>> _Firstly the interface isn't a layer of abstraction - it's a contract, a
boundary. An abstraction is an arbitrary concept which may or may not apply to
an interface._

That's just semantics, let's concentrate on why I would always want to have an
interface class instead of using the public class interface as the contract
between a class and the rest of the system using it. Remember that _many_
classes in _any_ software design don't have (or need to have) base subclasses,
composed classes, abstract interfaces, whatever. They are often nothing more
than pieces of data with related methods, and nothing else. Writing lots of
fluff around them to make such classes extensible, flexible, replaceable,
loosely coupled, easy to stub for testing, whatever, does not add any value
for you (as the programmer), your colleagues (when reading the code) or the
customer (when running it). You'll know it when you'll need to do all of this
anyway, and the moment you know it is early enough to think about it, no need
to have contingency plans for things your classes are not ever going to be
used for.

>> _Secondly, the abstract base concept is actually a deadly form of coupling.
As many people have suggested since the dawn of the problem, composition is
better than inheritance and what sits at the composition boundaries?
Interfaces!_

Interfaces, not interface _classes_. We have many classes in our codebase that
composite other classes, and none of them have interfaces at all, because the
whole codebase is C++, which doesn't even have the concept of interface
classes. Yet, our codebase is very clean and robust, fully covered by unit and
integration tests, and rarely (if ever) contains bugs that are caused by bad
or inflexible architecture. It does happen, on occasion, that some part of the
code or its design doesn't satisfy new requirements anymore, but when that
happens we simply refactor it, and if needed (which is extremely rare) write a
facade around it to support its old public interface.

Note that I'm not saying I'm happy with the fact C++ doesn't have interface
classes, because there definitely are situations I would want to use it, for
example where a component-based design would best fit the problem at hand. In
those cases we usually introduce a pure-abstract class that mimics an
interface class, which isn't ideal but does the job.

I'm not sold on the 'abstract base class is a deadly form of coupling' at all.
This may hold in some situations where you have many classes interacting in an
architecture or framework that has a high probability of changing/evolving
over time, but as I've said before: most code isn't like that. Most code is
actually pretty plain and boring, without much need for architecture at all.
Two or three levels in your class hierarchy and not more than a handful of
class dependencies can solve a surprising amount of programming problems.
Indiscriminately discarding inheritance or abstract base classes as 'evil' is
naive at best, ignorant at worst. I can come up with many programming problems
that are a perfect fit for concrete and/or polymorphic classes deriving from
(semi-) abstract base classes, that would most definitely not be better
implemented using composition.

>> _I have spent 2 weeks refactoring one into something which doesn't stick a
fork in your eye every time you change something_

Similarly, I've spent a whole lot more than 2 weeks refactoring big balls of
mud created by developers who threw as many classes against the wall as they
could come up with, introducing interfaces, delegates, composites, strategy
classes, and whatever other design pattern they found in the GoF book, only to
arrive at the conclusion that 80% of the code was boilerplate, and the
solution that was spread out over all those classes could be implemented by
1/10th the lines of code, concentrated in one or two classes. YMMV depending
on the application domain you work in, but having worked on large business
logic applications, web applications, and (now) scientific simulation
software, my experience is that most complexity in software systems is only
there because someone introduced it, not because it is necessary to solve the
problem it was supposed to solve.

>> _So your tests are chock full of empty stub classes rather than mocks?
Yuck._

Our test cases aren't full of stub classes, and they arent full of interface
classes either. Most classes can be perfectly tested in isolation by simply
instantiating them and feeding them with static inputs. Some others are
tightly coupled and covered by integration tests. None of this is rocket
science, we're at over 1M SLOC and we have no problems covering them in our
tests.

------
grapjas
How is any of this specific to the mess of the language that is Java?

~~~
konradb
This is true, it's essentially a summary of some of Clean Code by Robert
Martin. Principles applicable all over the place.

However to turn things a bit more constructive, let me know what languages you
see as less of a mess if you have the time. It doesn't matter whether we agree
and I'm not looking for a fight, just your opinion and reasoning. I might well
agree.

------
arethuza
Shouldn't KISS (Keep It Simple Stupid) be in that list?

Or at least avoiding "general-purpose tool-building factory factory"
factories:

[http://discuss.joelonsoftware.com/default.asp?joel.3.219431....](http://discuss.joelonsoftware.com/default.asp?joel.3.219431.12)

~~~
Jenk
The list as a whole covers KISS. Every (including those not on the list)
programming principle is aimed at the end goal of keeping things simple to
prevent maintenance headache.

~~~
cturner
"Don't repeat yourself" competes with KISS. DRY creates brittle code.

There's low harm in duplication of code. When something changes you don't have
to refactor your class structures. You just change the code for the thing it
affects. And if you have to make multiple edits it's often pretty obvious what
you need to do.

(Data and configuration are different, it's more important for these to have a
canonical home.)

~~~
Jenk
I strongly disagree. DRY does KISS. Much easier to maintain one instance of a
function/method/block of code than multiple copy/pastes. If something requires
different behaviour to what is in that extracted piece of code, then it
shouldn't be using the piece of code at all. Therefore KISS and remove that
reference. In conjunction with the other principles - delegate and
DI/Inversion in particular - it'll be easy to substitute the delegate and
minimise change.

~~~
limmeau
To me, that describes "maintainability", not "simplicity".

~~~
arethuza
Over the years I've seen a _lot_ of disagreement between developers as to what
"simple" means.

~~~
limmeau
Yes, and some definitions are wrong ;)

I invoke Layne's Law and go back to work.

~~~
arethuza
Perhaps the best example here being whether SOAP is simple or not - some
people think it is, others disagree....

------
moron4hire
I don't agree with most sentences on programming that contains the words
"prevent someone from".

"Classes, methods or functions should be Open for extension (new
functionality) and Closed for modification. This is another beautiful object
oriented design principle which prevents some-one from changing already tried
and tested code."

No, no, no. Let someone modify whatever the hell they want. If they screw it
up, that's on them. Quit it with this childish belief that you have to control
people.

~~~
tinco
Yes, but I think the closed principle is more about providing an interface,
i.e.: the interface should be closed for modification so that implementing
classes or interfacing services do not have to be changed everytime you add a
feature.

My girlfriend came up with a nice metaphor trying to follow along. The
open/closed principle applies to natural language as an interface. It is
important language can be extended to convey new meaning, but also important
that it is closed for modification so we retain the ability to understand
eachother.

~~~
moron4hire
With the exception of service APIs in which everyone is forced to use the
latest version, I still don't agree. If we're talking about libraries, nobody
is forcing people using the library to upgrade to the latest version (or at
least shouldn't be). If you have a good reason to change the interface, change
it, don't let the fact that it doesn't match previous versions of the
interface prevent you from improving the product.

------
mateuszf
Regarding rule "Object oriented design principle 8 – Interface Segregation
principle (ISP)" ... I think this is often overused. I only introduce
interfaces when there is more than one implementation. Of course if I'm
designing stable API this is different.

Also see: <http://stackoverflow.com/a/90915/343699>

~~~
mseebach
It can, especially to novice developers, be very helpful to be forced to think
of a class exclusively in terms of it's interface. Bad design seems to stand
out more.

~~~
qu4z-2
I would agree. Depending on implementation details tends to lead to much
tighter coupling between components.

------
jheriko
This is worded really quite badly, but makes a good set of points.

------
oddthink
One thing that was conspicuous for its absence was the "Law of Demeter." Is
this not considered a good practice any more?

It's trickier to use, because blindly applying it can lead to immense classes,
but I find it a useful thing to keep in mind.

~~~
ollysb
The rails delegate helper really helps with this, just declare which methods
you want forward, optionally with a prefix. Sometimes a different name is more
descriptive but this covers a large portion of the cases.

    
    
        class Foo < ActiveRecord::Base
          belongs_to :greeter
          delegate :hello, :to => :greeter
        end

------
jimmaswell
.equals ()? really? why not just ==?

------
martinced
Oh the memories. It's not very well written but most advices are sound if
you're stuck in that very particular Java / C# hell.

The book _"Effective Java"_ (from 2001) still has to one the "must read" for
any Java programmer that didn't read it.

In it Joshua Bloch has always been very clear, from the start, about all the
deeply ingrained ugly Java warts that Java had from the start (and still has).
Like for example the complete and total utter impossibility to respect the
equals() and hashcode() contracts in the face of inheritance.

Took a long time to set in but now quite some Java programmers are beginning
to accept that having equals() and hashcode() at the top of an inheritable OO
hierarchy may not have been the greatest thing since sliced bread.

The second book to read is Goets and al.'s "Java Concurrency in Practice" (one
of the best looking programming book cover ever, which never hurts an
incredibly high-quality content).

But the real lesson is that: Java is too complicated and has way too many
warts to ever produce beautiful code. The JVM is not all bad but "Java the
language" is really fugly.

Switch to a language that has a saner way to deal with concurrency (Say
Clojure's STM or Go) and you can throw most of the pages in these books to the
trash.

~~~
dougk16
Can you go a little more into the problems with equals() and hashCode(), or
link to something? I'm by no means a hardcore Java programmer, but I've used
it for several projects, and these never stood out as particularly problematic
to use and override, although I kind of see how they come across as kludgy in
a whitetower kind of way.

