Take the 2-minute tour ×

I am new in a enterprise level application project and discovered that 99% of everything I've seen in code is static methods, static properties.

The application at hand is a distributed app consisting of about a handful of WCF services that communicate through a business layer with a data access layer. The data access component is a singleton. There are transactions and MSMQ queues and multiple parties calling the WCF Services to notify the DB of changes, updates made in a seperate system and so forth.

First time running a little test project I got a thread related exception using EntLib5 Logging, telling me that some code was called from an unsynchronized code block.

I am curious to know how one should go about and improve an almost-all-static design.

  • Are there advantages to the current approach?
  • What are the disadvantages?
  • Where would you start (besides starting over) and improve all this mess?

Thanks for your opinions!

share|improve this question
I think this question should be moved. But one comment: I don't think anyone here can answer your questions. We've only have your report on the code and design. How am I to tell if there are advantages? BUt the lest one I can answer: start from scratch - you allone? Sorry but won't happen. –  Carsten König Sep 1 '11 at 7:20
fire lots of it! –  Jarrod Roberson Sep 1 '11 at 20:38
add comment

migrated from stackoverflow.com Sep 1 '11 at 15:15

This question came from our site for professional and enthusiast programmers.

3 Answers

Difficult question to answer, but the following should get you somewhere:

Static methods are not inherently evil. If they encapsulate some functionality then it's a completely valid way of doing something. In fact there are entire programming paradigms such as functional programming which require local state only.

However if there are any static state field or properties floating around then it's going to be problematic with respect to threading, whcih appears to be what you are experiencing as there is shared state.

I would:

  1. Lose Enterprise Library - it's a turd in many ways. Stick log4net in for logging. It's not intrusive like EL. It also hampers performance and has numerous threading issues itself.
  2. Find any static state variables and properties and attempt to make them method scoped. This will probably fix any threading problems related to shared state. If you have a static method, every piece of state that it touches or returns much go in and out via the parameters or return value.
  3. Turn all classes into non-static classes with instance methods.
  4. Try and isolate shared class dependencies into local read only variables (to be injected later).
  5. Extract interfaces from the classes and provide constructors which accept the dependencies.
  6. Use a container to construct the application instead of hard dependencies.

It's not going to be easy. Starting again if the codebase is relatively small is probably easier.

share|improve this answer
Please note that Enterprise Library is much more than a logging framework (it has blocks for validation, caching, cryptography, exception handling, etc). Because of this fact, you can't advice to ditch EL in favor of log4net, since that framework is solely for logging. –  Steven Sep 1 '11 at 11:33
However, I totally agree with you that "It's not going to be easy.". +1 –  Steven Sep 1 '11 at 11:33
Steven: I'd avoid it for everything TBH, although the data access is the least painful component. –  Deleted Sep 1 '11 at 11:43
Least painful and least useful :-). –  Steven Sep 1 '11 at 12:48
I'm a fan of the Validation Application Block. I like the flexibility. It allows many scenario’s that aren't possible with Data Annotations. However, it’s not an easy framework, that's for sure. –  Steven Sep 1 '11 at 12:48
show 2 more comments

When the application only consists of static methods it is clearly not object oriented (however, this doesn't mean that you should never have any static methods). Further more you will probably have little to no unit tests (since testing static things is almost impossible). The only advantage I can think of is that walking the call graph in the application is easy, because it is all static, but that's what where it stops.

I always favor a design around dependency injection, which enables unit testing, and enables you to plug in new functionality and cross cutting concerns (such as logging and security) more easily.

I still wouldn't exist to start all over again, because that is way too expensive (especially in an enterprise application) and it would be very hard to realistically estimate the time it would take (what would freak managers out) and you'll be wasting your bosses money.

No, instead, learn about object oriented design, unit testing, TDD, the SOLID principles, clean code, and dependency injection. After that, try improving the application in small chunks at the time. Try writing new code according to these principles and before changing old code (both for bug fixing and changing functionality) try to write automated tests for that part of the system. This way you can improve the code with more certainty.

It takes a lot of time to learn, and working this way takes a lot of time too. However, as you said yourself, you are in a mess, and not going out of this mess, means that at one point in time it will get impossible to change anything in the code base. The result will be that in the future you can make changes with more certainty and the system will get more maintainable and more resistant to change.

Good luck.

share|improve this answer
The inability to test static methods is a programming urban myth. As long as you write your static methods so that they have no side effects, they are easily testable. –  Robert Harvey Sep 1 '11 at 15:01
add comment

It sounds like very poor design. Let's look at it from a pure CLR point of view. There is little performance difference between instance and static object. However there will only one type object for a static class on the managed heap therefore you can't have proper object state, a singleton in effect to the call stack. Steve is right you also can't do any IoC and I suspect that threading will also become more of an issue. The design is very ridged and over time will increase cost of ownership, bad ROI. Code has to make money. If refactoring is an option then start at front of your app and start introducing instance classes and work back. This could end up being quiet a big composite refactor but you have little or no choice other than bin it. There are no advantages to having a static application. I had the same thing in a web application once, we rewrote large section of it and in the end binned it. GOOD LUCK.

share|improve this answer
add comment

Your Answer

