Damn it, can’t you make your code less complex?

By May 1, 2015 C# 10 Comments
shutterstock_132360455

<Rant>

oh horrorsI am so tired of complex code that can’t be read without having to trace definitions within definitions within references, within Interface definitions within indirection….  until I have no flippin’ idea what is going on.

This is even worse when perpetrated in sample code.  Can someone please tell sample code writers that the rule is “make it as simple as possible and demonstrate only one thing; and that one thing is not how clever you are.”

Here’s an actual line of code from an otherwise excellent sample program,

Let’s trace this just a bit,  IReadItemsByQueryRequest. This has a nice long name, but it doesn’t quite tell me what it does, so I’ll go to its definition. Guess what, it is an interface that derives from IBaseReadItemsRequest.  Following that thread we find out the base implements IBaseItemRequest and has an instance of IPagingParameters.  IBaseItemRequest uses four more Interfaces (!).

shutterstock_132360455Returning to our original thread, we see the use of requestMerger’s static method FillReadItemByQueryGaps.  This method implements an interface IReadItemsByQueryRequest and takes as a parameter an interface IREadItemsByQueryRequest.
Inside the method are three more interfaces to chase down. And so it goes
.

 

Good Code/ Bad Code

While this is “good code” – nicely architected with good separation of concerns and excellent naming (Uncle Bob would be proud) it is opaque.

There is zero chance of reading this and grokking it in less than a couple hours and a couple Advil.

And that is just one method chosen at random.

Please can we simplify? Even at the cost of making it more difficult to reuse the code (which is a myth 99% of the time anyway) can’t we make this code easier to follow?

Your outrage at my heresy  may be expressed below in the comments, but only after you decipher this sample application!

</Rant>

Is it us, or is it our Patterns?

PS, I’ve intentionally kept the author anonymous because I’m convinced she/he is an excellent programmer, who is just following the normal, modern patterns.  But perhaps it is time to re-examine those patterns.

The following two tabs change content below.

Jesse Liberty

Director of Technology Development at Falafel Software
Jesse Liberty is Director of Technical Development for Falafel Software, an author and he creates courses for Pluralsight . He is a Microsoft MVP, a Xamarin MVP and a Certified Xamarin Mobile Developer. Liberty hosts the popular Yet Another Podcast and his blog is considered required reading. He was a Senior Evangelist for Microsoft, Distinguished Software Engineer at AT&T; Software Architect for PBS and Vice President of Information Technology at Citibank.
  • Roman Hnatiuk

    I think the problem you are talking about can rather frequently be observed in live systems. The reason, as I see it, is developers’ “obsession” with OOP paradigms (to the limit), freely “spiced” with dependency injection. Once you have to dig into such a system, first thing you want to cry, and then commit suicide, as only perhaps 3% of the codebase does something needed by the task at hand, and everything else is code-water. I think it is time to write two more articles: “OOP Considered Harmful”, and “DI Considered Harmful”.

  • Adam Anderson

    That does sound a little extreme. I think my rule would be this: do not extract an interface until there is more than one implementer. Any sooner could be considered “premature architecture”. The practices described above have a good reason for existing (e.g. testability), but following the pattern slavishly without using it for its purpose just adds complexity without for no benefit.

    • Jesse Liberty

      I completely agree for production code but not for samples which should strip down the code to just what you need to understand how the SDK (or whatever) works.

  • http://sharpmobilecode.com/ Ruben Macias

    LOL, I enjoyed this post. I’ve worked with many of Uncle Bob’s loyal disciples and it’s a fine line between over architecting and making things only as complex as they need to be. My favorite quote was, “Doing it this way makes the code testable”. I then replied, “since when do you guys write unit tests??” (they didn’t).

  • Roy Cornelissen

    Great post and fully agree. The reusability myth bothers me quite a lot, I see so many over engineered code because of this. Add some AOP and DI Magic to the mix and you’re clueless as to what is going on.

    At Xpirit, where I work we have this adagium: Only add complexity if you need flexibility. Otherwise YAGNI.

  • Pingback: Jesse Liberty: Damn it! Can’t you make your code less complex? | XamGeek.com()

  • Pingback: Dew Drop – May 4, 2015 (#2006) | Morning Dew()

  • B. Clay Shannon

    Agreed – many “clever” (they are usually very intelligent) coders don’t understand that “the perfect is* the enemy of the good” (* or “can be” at any rate).

    Their “masterpiece” can become virtually unmaintainable due to its clever-clever tricks.

  • ardalis

    It’s best to follow Pain Driven Development (http://deviq.com/pain-driven-development/) and YAGNI. That said, some abstraction that eliminates tight coupling of code is almost always better than zero abstraction for any non-trivial example. And unfortunately when it comes to samples, the problem I see more often is not too much complexity, but too little. Most developers will take sample code and throw it right into production without realizing that it’s not meant for that. This is one situation in which a reasonable compromise might be to use comments to indicate where shortcuts have been taken in the interest of keeping the sample simple, noting that in a real application a more robust might be used.

    • Jesse Liberty

      good points, as always