3

I have just started working at a company where I have inherited a C# codebase from a previous developer. I know programming well, have an engineering degree + an (unfinished, several year long) PhD education in computer science and > 10 years of previous experience of "practical" programming, mainly in C++ and Java but also in other languages.

My problem is that the code that I have inherited is, in my opinion, absurdly over-engineered. I have enormous problems following the program flow and finding any concrete implementations of anything. The amount of abstractions are totally mind-boggling and there is no documentation whatsoever. Furthermore, it's difficult to debug the application since it's a server application which is split up into a "GUI-client" server and a "Control-Server" (also a relatively pointless abstraction since they always run on the same machine and there is a 1:1 relationship between them) which interfaces with industrial hardware which is not available.

To give an illustratory example, one of the classes have a field like this:

IClientFactory<IClient<ICommunicationService>> communicationFactory;

I have a hard time wrapping my head around it since the previous programmer also wrote this code (i.e. not using a bit-shift, and this is one of the few things that is documented):

public static bool IsBitHigh(int value, int bitIndex) {
    int valueAbs = Math.Abs(value);
    if (bitIndex < 0)
        throw new ArgumentException("Bit index must be above or equal to 0");
    int result = valueAbs & Convert.ToInt32(Math.Pow(Convert.ToDouble(2.0), Convert.ToDouble(bitIndex - 1)));

        return (result > 0);
    }

There are also indications that the previous programmer was surprised by the fact that casting floats to ints "rounds them down", and that's after several years in the industry.

So given this I have two questions:

  • Is this how code typically looks like in the industry? (I have an engineering/research background so I was under the illusion that people were still using for-loops.)

And more importantly:

  • Is there any easy way to reduce the complexity of the code, i.e. replace the interfaces with their concrete implementations? (To reverse-refactor the code in a sense.)
7
  • 1
    possible duplicate of I've inherited 200K lines of spaghetti code — what now?
    – gnat
    Commented Feb 26, 2015 at 16:02
  • see also: What is the point of having every service class have an interface?
    – gnat
    Commented Feb 26, 2015 at 16:04
  • 1
    Pretty sure he meant bitIndex to be greater than 0, since he subtracts 1 from it. Also, he could have done int result = valueAbs & (1 <<( bitIndex - 1)) and achieved the same result. The previous programmer doesn't strike me as very bright.
    – Neil
    Commented Feb 26, 2015 at 16:05
  • 1
    See this Programmers post for more information why code ends up like this.
    – gbjbaanb
    Commented Feb 26, 2015 at 16:22
  • Someone will link you to this, might as well be me... Michael Feathers, Working Effectively with Legacy Code. Does not matter if this code was written yesterday - it is legacy code - make this book your bible.
    – mattnz
    Commented Feb 26, 2015 at 20:35

1 Answer 1

5

My problem is that the code that I have inherited is, in my opinion, absurdly over-engineered. I have enormous problems following the program flow and finding any concrete implementations of anything. The amount of abstractions are totally mind-boggling and there is no documentation whatsoever.

Its not you, I find that a lot of C# and Java (and other "developer productivity languages" that also have tools for refactoring and 'twiddling' with code) have a preponderance of this kind of codebase, possibly because if its easy to write then there is plenty of time left over for tinkering. One company I worked for showed me a reference implementation of a webservice that had 1 method that took 1 parameter and returned a string... and was implemented using 32 C# class files in 6 assemblies. And to top it off there was a comment in one of them saying he would have used dependency injection but didn't want to over-complicate it!!! (A C equivalent would have been 32 lines of code)

So... I think the best thing to do is to work with it until you understand its layers, as I'm sure the previous developer managed to hold the overall structure in his head so he could work with it, with time you could get the same mental model of where all the bits are in order to maintain them.

An alternative is to understand the parts that fit together and consolidate them to remove much of the interfacing. eg your client-GUI and Control server can be merged but I feel this isn't the best place to attack - it might be a lot more work to add a control to the GUI (as you have to cascade the changes through both components) but that's common practice for n-tier development anyway.

Even with the morass of interfaces, they're probably OK overall. Maybe you need to consolidate some of them - not necessarily to their concrete classes as this won't give you much, but to remove much of the abstract plumbing that the interfaces were put in to support. ICommunicationService sounds like a good interface, a IClient<ICommunicationService> sounds unnecessary fluff, and getting rid of that might see IClientFactory going too as collateral damage.

All in all, you do what you can. But unfortunately it is somewhat common in the industry. Nobody understands KISS anymore :-(

5
  • 1
    It's my experience that programmers enjoy programming. I imagine that it is how builders feel when they build something wondrous. However unlike builders, it's all still abstract, and adding improvements where none are needed is like adding windows in the basement of a building.
    – Neil
    Commented Feb 26, 2015 at 16:36
  • 1
    @Neil True, and that's the problem - if only they enjoyed building the end-result, rather than building as an end in itself. Then maybe a few projects would get finished :-)
    – gbjbaanb
    Commented Feb 26, 2015 at 16:38
  • We all need to find joy in the elegance of how the program is coded, not just what it does. ;)
    – Neil
    Commented Feb 26, 2015 at 16:42
  • 1
    @Neil and the most elegantly designed programs are coded as if they were baroque cathedrals!
    – gbjbaanb
    Commented Feb 26, 2015 at 16:51
  • 1
    Well by elegant I don't mean sophisticated. I mean robust, simple, and easy to read.
    – Neil
    Commented Feb 26, 2015 at 16:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.