These are my personal top ten of software design evilness. That means ten designs that make your software either complex, untestable, monolithic, unchangeable or hard to understand:
- Enums (or other c# enumeration equivalents)
- Swallow exceptions
- God classes
- Missing interface segregation
- Monster classes
- Too configurable, too flexible
- Abstraction layers not providing additional abstraction
- Try catch for program flow control
- Missing logging
If you have more than 2 singletons in your code then I bet you’ll have problems maintaining your code. If singletons are accessed throughout your code then this has the following consequences:
- bad testability (mocking singletons in unit tests is a pain!)
- strong coupling (code calling singletons can not be used easily in another context)
- hard to read (dependency not visible on public interface of class/interface)
Singletons can come in disguise, too:
- static methods
- service locator: instead of calling a singleton, a service locator is used to get the “singleton”
My advice: use dependency injection (manually or with an inversion of control container)
BTW: The two singletons are the dependency injection container and the LogManager (if you are using log4net) 😉
Enums (or other c# enumeration equivalents)
Enums can be dangerous if:
- they have to be persisted: how to migrate old data into new version with additional values in the enum –> better use constants
- they define behaviour: not extensible with new behaviour without changing the enum (very bad in frameworks) –> use e.g. strategy pattern with different classes for enum values
If your code swallows exceptions then how should the clients of your code (the developers using your code or someone debugging your code) find what goes wrong? At least write a log message.
God classes are classes that know a lot about the system. For example they reference a lot of dependencies using their real type.
This design leads to a system that is very difficult to change and extend.
Missing Interface Segregation
Interfaces have to be written for the clients and nothing else. Clients should be provided with the functionality they need to accomplish their task but nothing more.
Violation of this interface segregation principle leads to a design that is hard to change because local changes tend to influence more than its local surrounding.
What can I write that you don’t know about monster classes? Perhaps that its a violation of the single responsibility principle (a class should do one and only one thing). Otherwise:
- Nobody feels at home in this thousands of lines of code
- Nobody will have the courage to change this code if it is needed, making things worse and worse and …
Too configurable, too flexible
Decide, decide, decide! I’ve seen a few systems that allowed to configure almost everything. Mostly because nobody could decide how the system should work: “We do not know what the system has to do, therefore make it configurable.”
The result can be a configurable or flexible system that
- is not configurable or flexible because only one single combination of all configurable/flexible parameters works (if it works at all!)
- is very complex although they solve only simple problems
- is not changeable anymore because it is unclear how new features affect the configurable parts
Abstraction Layers Providing No Additional Abstraction
Sounds senseless and is senseless. But it happens quite often. Here some samples:
- Abstraction layer for log4net (or log4j)
- Abstraction layer for O/R mapper
These abstraction layer are built because the developers hope that it will be possible to change the logger or O/R mapper latter without changing any of the code that uses this abstractions.
My experience is that if the logger or O/R mapper really changes that the interface changes are in a way that the abstraction layer interface has to be changed, too. Oops!
Try Catch For Program Flow Control
Using exceptions for program flow
- has bad performance
- is hard to understand
- results in very hard handling of real exceptional cases
A good logging concept is needed; otherwise
- debugging on production systems is nearly impossible (especially for random errors)
- the developers using your code have to use the debugger (if even possible) or a tracer to understand the flow of your code.
Please add your top design mistakes or thoughts in the comments!