Hacker News new | comments | show | ask | jobs | submit login
Object Oriented Design principles Java programmers should know (javarevisited.blogspot.com)
60 points by javinpaul 1609 days ago | hide | past | web | 57 comments | favorite



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.


There are no absolute truths in coding, so don't pretend there are. These are classic rules of thumb that are good to read every once in a while. Take from them what you like, leave the rest alone.


I prefer to think about these rules in terms of bets.

Adding e.g. a symbolic constant is a bet. You win if during the lifetime of the system, you discover that the constant must be changed, and the gain is the time saved by changing it quickly and with few errors. If you never have to change the constant, you lose the time spent on introducing the constant and the time spent on looking up the constant while reading the code.

Most of the rules here are long change (or whatever the financial people call betting on something), except for KISS.


I agree. Also, extracting to methods and constants is long on on the idea that the code might be harder to understand without meaningful symbols in the future, which is a pretty safe bet usually :)


So that there are no absolute truths in coding.. is an absolute truth in coding? Please don't pretend to be able to tell me what to pretend, I don't think it's conductive to the discussion.

I don't think these are rules of thumb that you should take for granted without criticism, especially when paraphrased from their original (well researched) source.

And I especially don't think you should take from them what you like. They are principles you should know because they are known to lead to well designed software, you should leave none of them alone.


> Take from them what you like, leave the rest alone.

The person you responded to mentioned novice programmers. Teaching novice programmers with the attitude of "use as much of this as you like/want/understand" is a pretty poor technique that will not give good results.


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.


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.


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.


Originally it was ""if you have block of code in more than two places", tinco removed that from their reply.

I believe that inclusion solves the issue you've described. Obviously you don't need every 5 lines of code encapsulated into a method, but preventing code duplication should be paramount.


While it's not necessary to pull every block of code out into it's own function, its something a developer should keep in mind when writing code.

A lot of people seem to thing that code duplication is the only reason to refactor code out into it's own function. While that's certainly a good reason, there are many others. For me, being able to test the code easily is a major one. The smaller the function (specifically the less branching and the less it does), the easier it is to test.

Obviously, it's a balancing act.


When you consider an act, you decide whether or not to perform that act. Consideration does not imply causation.


I think the wishy-washy wording you see are the result of a non-native writer relying on cliches to overcome their lack of skill in English. It's easy for an unskilled person to sound arrogant or rude when they try to be firm and concise.


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


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.


Agreed. There is a fine line on how much of this should be used. Teams can spend cycles trying to design everything based on IoC which might be overkill. All rules regarding keeping things simple should also apply to IoC.


Agreed, I think IoC is dramatically overrated along with AoP. There are places it can be useful but it can get so complicated so quickly it actually hinders the upkeep of the application.


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.


Which has absolutely nothing to do with OO specifically.


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.


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.


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.


YAGNI serves me more often than it hurts me, in this case (as with all such rules-of-thumb, they are double-edged-swords, to mix my metaphors). I try not to write code I don't need until I need it. If an interface is necessary, it'll come out eventually. But I agree, some people get really abstraction happy.


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.


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


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


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


:) I can agree that it is implicit. I just want it explicit.


YES! A corollary to this: there are way too many other things to do to obsess over one code base. I hate the concept of "code ownership" as it invariably leads to "arguments when you change 'my' code".


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


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.


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


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.


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


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.


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


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


Yes, and some definitions are wrong ;)

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


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


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.


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.


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.


The open-closed principle involves creating classes that _never_ change in the future. If you want to extend their behaviour you should subclass them. This makes some sense in a codebase with no tests, the risk of change becomes too high. But then who codes without tests these days? Given that you should always be writing tests for your code I think this principle should probably be left in the history books. It seems like a great way to add unnecessary complexity to your code base.


> Let someone modify whatever the hell they want. If they screw it up, that's on them.

If changing a common component or library breaks my code, they may be at fault, but it's still my feature that breaks. The open-closed principle is about sandboxing modifications to the scenarios that need those modifications. It's not about babysitting.

Without it, when you make a change you need to consider every consumer or risk breaking their case. Or, you get these nightmarish components that have "modes" (if I'm included in this context, act like this, if I'm included in that context, act differently).


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


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.


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


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


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.


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


My friend-group has taken to calling it the Sometimes Helpful Suggestion of Demeter. It's sometimes helpful, but as you say, not really a "Law".


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


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.


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.


"Like for example the complete and total utter impossibility to respect the equals() and hashcode() contracts in the face of inheritance."

But is that a problem with Java or is it a problem with ANY OO language?


One additional item I would suggest is Fowler's Refactoring. In java hell, the only viable weapon is a refactoring scalpel.


I highly recommend the second edition of Effective Java, which is more recent (2008) and covers Java SE 5 & 6.




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact

Search: