Improving The Code I Write

The main reason I write articles is so that I have a place to put all the cool things I find regarding coding in one place. Something has recently been coming up that has captured my attention: “Code Smells”. By Code smells, I mean the ability to look or evaluate any piece of code that has been written and assess its value based on a number of indicators.

The question that started me on this path is: “How many developers look at ways of improving the way they write code?”. I have been writing recently to try and learn and refresh my knowledge on technologies as a way to improve the code I write. BUT this brings up a big question, does knowing a lot of technologies make you a better coder? I would assume that the more you know makes you more valuable since the company you work for would not need to train you, but does this make you a better coder? I don't think so because you could still write messy code that is difficult to maintain.

So in my quest to better the code I write, I have been looking at ways of improving and making sure that code I am writing does not have the following smells:

Duplicate Code

One of the best indicators for me is code duplication. I am sure that people add duplicated code into an application for many reasons: not enough time to extract the logic into a common method library, ease of copy and paste, etc. The reason I put this at the top is that it when you come to maintain an application that has a lot of code duplication or duplicated logic, it show on the amount of time it takes to do simple changes. Many times, changes that have been made to a system will not have been updated in all the duplicated methods and lead to easily avoidable bugs being introduced. The Principle DRY: Don't Repeat Yourself is a good way of making sure when you are coding to try and look at the bigger picture. It may take a little longer when you are initially writing the code, but I promise it will save time in the long run.

The God Object

This is something that can creep into any application. My guess is that the coder is trying to make it simple and keep related logic in one place or the team is being lazy when adding to existing factories/objects. The problem with this is that you end up with a method that does EVERYTHING and again becomes a nightmare to maintain. I have seen coders propose frameworks that would be guilty of this, that down the line cause maintenance teams endless hassles.

Overly Long Methods

This is closely tied to the god method I mentioned above. It refers to methods with too much logic and there are hundreds of lines long. Obviously when another developer comes along, they struggle to understand what the purpose of the method is and how everything works. A solution to this would be to try and break up the method into components that help make it more manageable.

Large Chunks of Commented Out Code

I haven't seen too many complaints regarding this, but it I can't stand it when I see it. What is the point of leaving code commented out for a considerable time. I obviously don't mind code commented out if it is causing a problem or will be reinstated after a short period. But large chunks of code commented out for large periods of time makes no sense. Most projects I hope are in some kind of source control. If that is the case, the developer could go back, find the code if it is needed again and correctly labelled / branched?

Global Variables

Global variables can be bad for many reasons, I will relate the issues I have had with them. The reason they show bad practice is that they can be accessed and changed from any method. They can be changed anywhere not to mention shadowed and increase the likelihood of bugs. Please don't misunderstand me, I have no problem with properties on web forms. The issue I am talking about would be variable that is used by multiple methods, i.e. a Data Set that get set and reset in different places.

Throwing an Exception in a Catch Block

I have never really understood this practice. If you need to make sure that you are closing streams or there is code that needs to run, you can and a finally clause. The code I am talking about is:

try
{
    throw new Exception("Test");
}
catch (Exception exception)
{
    throw exception;
}
finally
{
    //Code that needs to run
}

Re-throwing the exception destroys the stack trace information and causes headaches when trying to debug the error.

If you need to log the error you could use the following to re-throw the error without destroying the stack trace:

try
{
    throw new Exception("Test");
}
catch (Exception exception)
{
    Logger.LogError(exception)
    throw; //Preserves the stack trace
}
finally
{
    //Code
}

Non Descriptive Variable Names

This has to be my pet hate. In a few projects, I have looked at the naming convention seems to be the use of single char variable names as in int a; string b;. This isn't minified JavaScript, this was working C# code. There must be some reason people do it, maybe they want people to not meddle with their code or make it hard to understand. Personally I stay away from code that does this simply because making any changes could cause errors. Not to mention that trying to remember what variable is what or used for what can be difficult.

I was going to add Non descriptive Method names as in two or three letter method names but I think it could all go under the same category since it is really the same thing. I think it goes without saying that while reading through code that references the method ssd which gets back the variable w and goes on to use it in a method named Adw does really go a long way in making the code easy to understand.

In closing, the reason I am bringing up these points is that I want to have pride in the work that I do. I do see it as a work of art. Not only are we creating things, but they are useful (hopefully) to people that need to get their job done.

I am always keen on suggestions and ways of keeping my code clean and practices that aid this, so if you have suggestions please reply to this post.

CodeProject