Home » Programming Best Practice » Avoiding Code Complexity

Avoiding Code Complexity

clip_image001A 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.

 

Other post in Programming Best Practice

About Dave Bush

Dave Bush is a Full Stack ASP.NET developer focusing on ASP.NET, C#, Node.js, JavaScript, HTML, CSS, BootStrap, and Angular.JS. Does your team need additional help in any of the above? Contact Dave today.

Leave a Reply

7 Comments on "Avoiding Code Complexity"

Notify of
avatar
Sort by:   newest | oldest | most voted
trackback

[…] Avoiding Code Complexity (Dave M. Bush) […]

Kevin O'Shaughnessy
Guest
I am always encouraging other developers to split up longer methods. Often long methods hide bugs because they are much harder to spot amongst a lot of other code. WET code often hides in long methods and by splitting into smaller methods you can find the repetition and eliminate code. I think limiting every method to only a maximum of mine lines is a little extreme though. I think it’s more important that the method name accurately reflects what it does and is easy to understand. This usually means it performs some sort of business function. That is sometimes hard… Read more »
Dave Bush
Guest
As I said in the post, 9 lines should be the norm. There are times when this may not be achievable. But, as you’ve already agreed, the shorter we can get our methods, the easier they are to read, the fewer bugs will be introduced, and the less repetition of code will be introduced. I was reviewing a pretty long method in code that I wrote recently, and while there wasn’t any code duplication, I quickly realized that if I had broken the code into smaller chunks, it would be much easier to read and wouldn’t need any comments. If… Read more »
Kevin O'Shaughnessy
Guest
Is there are science to say that more than 9 lines is too many to be readable? Much of my work is with ASP.NET MVC. Like most developers I inherited a lot of code written in a different way to how I would have gone about it. Each model inherits an interface with a CreateModel method. Some of these methods are very long. Some of the models are around 1000 lines of code. I have been splitting the long methods out. But I wonder if this should all be done in one class? Yes it is all the same model,… Read more »
Dave Bush
Guest

https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two

1000 lines of code in a model?!! Aren’t models mostly properties that represent fields in a database (or something similar). Sounds like the class could be broken up. Without seeing the code base, it is hard to say. Some questions to reflect on:

Have they mixed the model with business rules?
Is the create method doing essentially the same thing over and over again and you might be able to abstract it into the base class using either reflection or generics?

Kevin O'Shaughnessy
Guest
I read about Miller’s law in Code Complete, but McConnell reached different conclusions. I might write blog post about it later. Are there business rules in the model? What are business rules actually are is a little subjective, but I would say yes. There seem to be two schools of thought on whether there should be any business logic in your model. The way I understand it is the original MVC pattern from the 70s was designed so that the business logic would be in the model and the controller rather than the view. Would you say all the business… Read more »
Dave Bush
Guest
Actually, I tend toward the concept of “services” for business rules. I first saw this in AngularJS, but it would transfer pretty nicely to ASP.NET MVC or MVP. MVC just states where the model, the view and the controller live and how they interact with each other. The model, is the resulting data that the view will need to represent and the controller is the traffic cop that gets the data between the view and the model. All of that can, and probably should sit on top of other code that would be more typically represented as tiers with the… Read more »
wpDiscuz