DevOps Zone is brought to you in partnership with:

Dave Bush is a .NET programmer and Certified ScrumMaster who is passionate about managing risk as it relates to developing software. When he is not writing or speaking about topics related to Application Lifecycle Risk Management (ALRM), he is an example to his peers as he develops web sites in the ASP.NET environment using industry best practices. Specific topics Dave can address include: • Project management, with an emphasis on Scrum • Test Driven Development (TDD) • Behavioral Driven Development (BDD) • Unit testing and Integration testing using NUnit, Jasmine and SpecFlow • Web Application testing using Selenium • Continuous Integration • Extreme programming (XP) • Coding best practices • Architecture • Code Reviews Dave has "an insatiable curiosity and is always learning." He has been called "the miracle worker" and "hard to replace" by clients he has worked for recently. Contact Dave via LinkedIn (http://www.linkedin.com/in/davembush/) to find out more about how he can help your organization reduce software development risk Dave is a DZone MVB and is not an employee of DZone and has posted 56 posts at DZone. You can read more from them at their website. View Full User Profile

On Avoiding Code Complexity

06.27.2014
| 11650 views |
  • submit to reddit

A couple of weeks ago, I talked a bit about how we name things.  Specifically, I talked about the very bad habit of using the variables i, j, and k as variables in our for/next loops.

A few weeks before that, I talked about DRY Programming and the fact that not repeating ourselves extends much farther than most of us normally think when we are thinking about our code.

Today I want to continue on the general theme of code quality by discussing code complexity.

What is it?

Generally, code complexity is anything you introduce into your code that makes it hard to follow.  But here are a few areas you might look for code complexity.

Do you have REALLY long methods?

Here’s the deal.  The longer your method is, the more work it will be to keep track of the overall point of the method.  So, you want to keep the number of lines in your method low.  If you are doing too much, when you come back to your code, even you will be confused.

I remember when I first started programming.  We only had 25 lines for a full screen, and the rule was, you should be able to see the full function or method on one screen full of code in your editor.  The problem with that metric now is that our screens have gotten capable of showing a lot more code.  It’s a lot like telling a new driver, he should be able to still see the license plates of the car in front of him when he stops behind a car.  Some cars exist where my tiny little Civic would be IN the other car and I’d still be able to see the plates.

A better metric would be to use the 7 +/- 2 rule.  Ideally no more than 7 lines of functional code per method.  If you have to, you can fudge it to up to 9, but no more.  This is because the human brain can only deal with about 7 items at a time.

I can think of a few times when you might want to break this rule, like when you have a set of conditions that really need to all be in the same place for them to make sense, but it wouldn’t hurt to try to keep to the rule as often as possible.

Do you have a lot of conditions within one method?

When I was discussing the problem with using i, j, and k as variables, I kind of mentioned this, but I didn’t dwell on the subject a lot.

You see, the story I told when I was telling you all about the i, j, and k problem violated all of the readability rules.  First, it was using the wrong variable names.  Second, the method was MUCH too long.  And third it had too many conditions.

You might think that only if/else statements are conditions.  But so are for/next, while, do/while, and switch statements.  As much as you can, your code should be setup so that you only have one condition per method.  Three at the most.  Again, switch statements might loose their context, but here, I would have one method that handles the switch statement and only one line per case statement.  Each case statement should call a function that does the real work.

There is a tool in Visual Studio 2013 that will help you determine how bad your methods are.  You can calculate metrics for a project or the entire solution and it will generate a table of the Cyclomatic complexity of your code.  Look for methods that have a Cyclomatic complexity of over 10 and try to bring them down.  The closer to  zero you get, the better.  Many people suggest that we keep this number below 10 or 11, but this is just an opinion.  I would rather just say look at what you’ve currently got and strive for better.

If you have Visual Studio, this option is under the Analyze menu option.

Do you have a lot of operations, function calls, parameters to those calls?

What I’m basically talking about here is what’s called Halstead Volume or Halstead Metrics.  What this computes is how complex the code is.

For example, if you have five lines of code and a Cyclomatic complexity of ten as everyone suggest, your code may still be in trouble because each line of code is so complex that no one on earth could possibly understand it.

We call this self obfuscating code.

You probably have some superstar on your team that thinks he’s so hot that he codes an entire function on one line.  The problem with this is that six months from now, neither he nor you will be able to figure out what the code does.  Any fix that will be needed will require an entire rewrite of the code.  That’s exactly what we are trying to prevent.

If you are using Visual Studio 2013 Ultimate, you can get the Microsoft Codelens Code Health Indicator which adds the ability to check for all three of the above problems for each of your methods.  If you pay attention to it, it will help you make code that is easier to understand.

Can You Easily Find Code You Need To Maintain?

This is one that is harder to detect, but I thought I’d mention it here because it is a real issue.

In an ideal world, we shouldn’t need to use a debugger to track down where the code is that we need to modify.  There are cases where this may be the only solution.  But you should know, for example, that all of your validation code is in this one location.  If you have validation code in multiple locations, you are probably thinking about your code incorrectly.

But also cut yourself some slack because this kind of code complexity is not something you are going to notice until late in your project.  You may never see it, but you will certainly recognize it when you run into it in someone else’s code.

Which reminds me of a story.

There was this guy that was working in a cube and he suddenly starts ranting.  “Who wrote this code?!  This is the dumbest code I’ve ever seen.  etc…”  Suddenly he got real quiet and the guy in the next cube asked him, “hey, Joe, what’s wrong?”  Joe replies, “It’s my code.”

So even if you don’t pay attention to these issues for your coworker’s sake.  Do it for yourself.

Published at DZone with permission of Dave Bush, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Lund Wolfe replied on Sun, 2014/06/29 - 10:13pm

"Can You Easily Find Code You Need To Maintain?"  That's how I define complexity.  Is it obvious what the application does and how it does it ?

Pierre-yves Saumont replied on Wed, 2014/07/02 - 7:46am

Having a big method of more of 100 lines, or having 10 calls to 10 private methods of 10 lines may be exactly the same if each method does something else than returning a value (for example mutating its arguments or mutating the enclosing scope).

BTW, measuring complexity by the number of lines is not very efficient. This line of code:

remoteAgentMap.get(clientId).flatMap(f0.apply(configurationParameters)).map(f1).flatMap(f2).flatMap(f3).map(f4).foreachOrFail(Runnable::run).foreach(logger::info);

may be rewritten as:

remoteAgentMap
    .get(clientId)
    .flatMap(f0.apply(configurationParameters))
    .map(f1)
    .flatMap(f2)
    .flatMap(f3)
    .map(f4)
    .foreachOrFail(Runnable::run)
    .foreach(logger::info);

It is probably clearer, but it is only a question of formatting.

This is simple code (in either cases) because all functions (f0 to f4) are pure. (They do nothing except returning a value). This code has a cyclomatic complexity of 1 and is simple to understand. It looks for a value in a map, then binds the result with a bunch of functions, eventually producing something to be run (in case of success) or an error to be logged (in case of failure).

What the code actually does is irrelevant. It depends only on the functions that are bound.

In my opinion, reducing complexity is all about abstracting control structures from business code. Think about how easy it would be to test code with no control structures at all! (no if..then...else, no for...next, no while...)

This is what functional programming is about (among other things).

Raging Infernoz replied on Wed, 2014/07/02 - 6:32pm

The number of lines is irrelevant, needless and disassociated complexity is what matters.

e.g. you could have a very long switch statement which has under 10 lines of code per case, for a very long list of record types or commands, and that is the simplest and easiest way to write this; I have done this, because it made more sense and better optimised code than an ugly attempt at a command pattern including e.g. via more bloated hijacking of enum methods!

e.g. You could have a long algorithm which can't be easily split, and to try to do so will make the code harder to understand, because you run out of mental bandwidth jumping between other methods/classes; I've seen this and felt the pain!

One of my pet hates is not method bloat, but rather too much inheritance and stuff in a java file, like distinct functionality which should have it's own class, and a class bloated with inner classes which should be in their own java files, possibly in a sub-package.

I think this quote make sense, possibly based on an Einstein quote, possibly based on Occam's Razor:

"In every field of inquiry, it is true that all things should be made as simple as possible – but no simpler. (And for every problem that is muddled by over-complexity, a dozen are muddled by over-simplifying.)"

Rusi Popov replied on Thu, 2014/07/03 - 3:23am

Maybe within the scope of a single method what actually matters for its understanding and perception of its complexity is the number of objects (variables, constants, parameters, fields, methods, entry/exit points) the reader should keep in mind in order to understand "a reasonable fragment" of it and also the number of such fragments. Such a fragment could be a block in the terms of the syntax of the language used (like between begin-end keywords, { and } or considering the same nesting). The number of blocks also increases the complexity. The analysis could be based on the data flow diagram and dependency tracking.

Thus, methods that could be split in a sequence of independent blocks could be read and understood block by block, so then only the number of such blocks matters, but not the lines in all of them. Most probably then that number could be either the Miller's number 7+/-2.
In addition, if up to 7+/-2 blocks could be composed in a single block and understood as a single concept, then they could be understood as a single object, increasing the complexity just by 1.

Thus, the complexity of the code Pierre-yves Saumont could be:

10 (number of variables) + 9 (number of methods)

But blocks like i++;i++;....;i++; (no matter how many times or lines) could have complexity of 1.

Also important thing is the number of deviations from the coding standard and conventions adopted (by the reader), increasing the complexity.

Pierre-yves Saumont replied on Fri, 2014/07/04 - 2:31am in response to: Rusi Popov

There are no variables in my example. Only constants. Functional programming does not use variables.

Rusi Popov replied on Fri, 2014/07/04 - 4:43am in response to: Pierre-yves Saumont

If functional programming is used or not, this is not important. The complexity comes from the fact that in order to understand what that example statement does, one should keep in mind what remoteAgentMap, clientId, f0, configurationParameters, f1,f2, f3, f4,Runnable::run, logger::info, mean.

They still fit in the definition above "... the number of objects (variables, constants, parameters, fields, methods, entry/exit points) the reader should keep in mind in order to understand..."

Rusi Popov replied on Fri, 2014/07/04 - 4:49am in response to: Pierre-yves Saumont

Pierre-yves Saumont replied on Fri, 2014/07/04 - 10:37am in response to: Rusi Popov

I am not sure that complexity is proportional to the number of "objects". The following code:


personMap.find("Name") .flatMap(Person::getAddress) .map(Address::getCity) .ifPresent(ThisClass::process)

has less objects than this version:


Optional name = personMap.find("Name"); Optional address = name.flatMap(Person::getAddress); Optional city = address.map(Address::getCity); city.ifPresent(ThisClass::process)

For some people, the second version, with more objects, is easier to understand. Some objects only add additional information that helps understanding the code. For others, this additional information is only noise that make the code more difficult to read. In fact, we are not speaking about the same thing. Perceived complexity is subjective and varies with the reader. Chinese may be more complex than English, although not for Chinese people.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.