What are the tell-tale signs of bad object oriented design?

Question

When designing a new system or getting your head around someone else's code, what are some tell tale signs that something has gone wrong in the design phase? Are there clues to look for on class diagrams and inheritance hierarchies or even in the code itself that just scream for a design overhaul, particularly early in a project?

Answer 1

One thing I hate to see is a base class down-casting itself to a derived class. When you see this, you know you have problems.

Other examples might be:

• Excessive use of switch statements
• Derived classes that override everything

Answer 2

Here's a few:

• Circular dependencies
• You with property XYZ of a base class wasn't protected/private
• You wish your language supported multiple inheritance

Answer 3

The things that mostly stick out for me are "code smells".

Mostly I'm sensitive to things that go against "good practice".

Things like ...

... Methods that do things other than what you'd think from the name (eg: FileExists() that silently deletes zero byte files)

... A few extremely long methods (sign of an object wrapper around a procedure)

... Repeated use of switch/case statements on the same enumerated member (sign of sub-classes needing extraction)

... Lots of member variables that are used for processing, not to capture state (might indicate need to extract a method object)

... A class that has lots of responsibilities (violation of Single Repsonsibility principle)

... Long chains of member access (this.that is fine, this.that.theOther is fine, but my.very.long.chain.of.member.accesses.for.a.result is brittle)

... Poor naming of classes

... Use of too many design patterns in a small space

... Working too hard (rewriting functions already present in the framework, or elsewhere in the same project)

... Poor spelling (anywhere) and grammar (in comments), or comments that are simply misleading

Answer 4

In my view, all OOP code degenerates to procedural code over a sufficiently long time span.

Granted, if you read my most recent question, you might understand why I am a little jaded.

The key problem with OOP is that it doesn't make it obvious that your object construction graph should be independent of your call graph.

Once you fix that problem, OOP actually starts to make sense. The problem is that very few teams are aware of this design pattern.

Answer 5

Having all you objects inherit some base utility class just so you can call your utility methods without having to type so much code.

Answer 6

Impossible to unit test properly.

Answer 7

Check this out. Its awesome.

Bad Smells: http://c2.com/xp/CodeSmell.html

Its not specific to object oriented programming, but it covers a lot of bits. Cheers.

Answer 8

I'd say the number one rule of poor OO design (and yes I've been guilty of it too many times!) is:

• Classes that break the Single Responsibility Principle (SRP) and perform too many actions

Followed by:

• Too much inheritance instead of composition, i.e. Classes that derive from a sub-type purely so they get functionality for free. Favour Composition over Inheritance.

Answer 9

This question makes the assumption that object-oriented means good design. There are cases where another approach is much more appropriate.

Answer 10

One smell is objects having hard dependencies/references to other objects that aren't a part of their natural object hierarchy or domain related composition.

Example: Say you have a city simulation. If the a Person object has a NearestPostOffice property you are probably in trouble.

Answer 11

Find a programmer who is experienced with the code base. Ask them to explain how something works.

If they say "this function calls that function", their code is procedural.

If they say "this class interacts with that class", their code is OO.

Answer 12

Within a long method, sections surrounded with #region / #endregion - in almost every case I've seen, that code could easily be extracted into a new method OR needed to be refactored in some way.

Overly-complicated inheritance trees, where the sub-classes do very different things and are only tangentially related to one another.

Violation of DRY - sub-classes that each override a base method in almost exactly the same way, with only a minor variation. An example: I recently worked on some code where the subclasses each overrode a base method and where the only difference was a type test ("x is ThisType" vs "x is ThatType"). I implemented a method in the base that took a generic type T, that it then used in the test. Each child could then call the base implementation, passing the type it wanted to test against. This trimmed about 30 lines of code from each of 8 different child classes.

Answer 13

Anti-patterns

Software design anti-patterns

• Abstraction inversion : Not exposing implemented functionality required by users, so that they re-implement it using higher level functions
• Ambiguous viewpoint: Presenting a model (usually OOAD) without specifying its viewpoint
• Big ball of mud: A system with no recognizable structure
• Blob: Generalization of God object from object-oriented design
• Gas factory: An unnecessarily complex design
• Input kludge: Failing to specify and implement handling of possibly invalid input
• Interface bloat: Making an interface so powerful that it is extremely difficult to implement
• Magic pushbutton: Coding implementation logic directly within interface code, without using abstraction.
• Race hazard: Failing to see the consequence of different orders of events
• Railroaded solution: A proposed solution that while poor, is the only one available due to poor foresight and inflexibility in other areas of the design
• Re-coupling: Introducing unnecessary object dependency
• Stovepipe system: A barely maintainable assemblage of ill-related components
• Staralised schema: A database schema containing dual purpose tables for normalised and datamart use

Object-oriented design anti-patterns

• Anemic Domain Model: The use of domain model without any business logic which is not OOP because each object should have both attributes and behaviors
• BaseBean: Inheriting functionality from a utility class rather than delegating to it
• Call super: Requiring subclasses to call a superclass's overridden method
• Circle-ellipse problem: Subtyping variable-types on the basis of value-subtypes
• Empty subclass failure: Creating a class that fails the "Empty Subclass Test" by behaving differently from a class derived from it without modifications
• God object: Concentrating too many functions in a single part of the design (class)
• Object cesspool: Reusing objects whose state does not conform to the (possibly implicit) contract for re-use
• Object orgy: Failing to properly encapsulate objects permitting unrestricted access to their internals
• Poltergeists: Objects whose sole purpose is to pass information to another object
• Sequential coupling: A class that requires its methods to be called in a particular order
• Singletonitis: The overuse of the singleton pattern
• Yet Another Useless Layer: Adding unnecessary layers to a program, library or framework. This became popular after the first book on programming patterns.
• Yo-yo problem: A structure (e.g., of inheritance) that is hard to understand due to excessive fragmentation