How many levels of indirection before we consider it too many?

So I had a scenario today where I had to look through some code written by another team. Unfortunately everyone who had worked on the code in the past had left. The code was well written, the problem was with the amount of levels of indirection.

Don't get me wrong, I like well formatted code, with strong design patterns and solid principles. Indirection is great for testability, but you can definitely take it too far!

The problem I faced was a "URL Helper", whose job was to return a url based on a key.

No biggie.

The key was a variable, retrieved from a service called "ResourceHelper", which is a service that returns a string based on an id.

Annoying,  but still no biggie. I guess the key could change?

The url was being passed into a generic Api caller, a few parameter strings were being passed in, but nothing to really give the game away.

More annoying, as I don't know what's being called,  but maybe the request contract changes frequently?

The generic Api caller returned a dynamic object, of which a value resultcode was being extracted.

This is starting to get a bit unnecessary now,  but at least I'm kind of following it? Does this mean there is no response contract or, maybe the implementation of the service contract could change?

I followed what happened after the call to the service for a clue, but the page then redirected to the same page,  but appending the result code to query string.

I gave up. I stopped and tried to work out why someone would do this, or whether they are just messing with my brain!

So the page is calling a service, of which I don't know the url, by pulling the url back from somewhere using a key that could change or at least I don't know right now, to return a value that's called resultcode that doesn't have a contract,  only to reload the page?

I was annoyed but carried on anyway. I realised the url helper was getting the url from the config and found a key that vaguely matched the variable name in code. The url was something like http://stubservice?return=1

Again no clue.

I questioned why would someone create such level of indirection? What's the point? What are they trying to achieve? Who is it benefiting? Are they purposefully trying to obfuscate the code? Why me? What were they thinking? Were they having a breakdown? Is this deception?

I ended up finding out the solution in the end, by looking at the environment configuration in an integrated environment,  and looking at the url it had in config.  Completely uncessary. I hastly refactored the code, adding comments where I couldn't refactor, praying I'd never have to look at code like this again.

0 mentions:

Post a Comment