Clean Errors – Clean Code

If you write feature code, you also have to write error handling code. There’s no clean code without clean error handling, if you don’t do it, your user has to deal with it in the form of your application crashing.

There are a few simple rules you should follow to, starting with using exceptions rather than return codes, we already talked about it when dealing with clean functions. Using unchecked exceptions let’s you concentrate in the flow of your code without interweaving it with a myriad of if-else statements to check every return code (or worse forget to do it and propagate an error state).

You should throw your exception thinking about how your caller will deal with it. So you need to provide context about the error in the message of the exception, and don’t litter your caller with very specific exception classes, if all of them will be dealt with in the same way (log and show error).

Remember that exceptions should be used to handle the weird flow not your normal flow. Don’t use them like if statements. If you have to integrate with a library that does just that wrap it and expose a sane interface that the rest of your system can deal with.

Avoid nulls like a plague. This probably deserves a whole article but to give you the gist of it, don’t return null, return something else, like an empty list or a special case object. Don’t pass null as a parameters. You don’t want to deal with it neither the writer of the function you are calling (likely yourself), don’t pass a null as an argument, unless it’s a function of a third-party library.

Advertisements

Clean Formatting – Clean Code

You just need to have a look at any minified JS file to realize how important proper formatting is.I don’t think any one would be able to program a whole system constantly minified, but most don’t give a concerned effort in improving the formatting of their code base, not even using the autoformatting features of the IDEs.

So in order of effort:

  1. If you are using Visual Studio type periodically Ctrl+K, Ctrl+D. This will auto format the current file giving you a nice starting point
  2. Install Code Maid. It’s a nice Visual Studio extension that helps formatting your code, you can configure it to do many little things, like grouping methods by its visibility (public, protected, private), removing unused using statements. It’s more powerful than the standard reformatting, and can also be configured to clean up at every save.

Code Maid goes a long way in improving your formatting, but when working on a team, it’s important that most of the team members use it, if not on auto mode, at least before any check-in. Otherwise, the commits done by the team members that use it risk being dirty with a lot of reformatting.

One thing I wasn’t able to do with code maid was enforce the news paper order to my methods. What’s that? Well, you see, a news paper article is organized in a way that first it gives you broad idea, then it goes a little bit in detail, then a little more, and then you get to the full picture.

In code you have high level functions calling low-level functions. So the same principle should apply. At the top of the file you should find the high level methods, then you should go down a level, and keep going till it the end.

Code maid isn’t smart enough to look at who calls who and order the methods with that in mind, but you have a brain and should use it 🙂

Other rules you should keep in mind are:

  • Add vertical spacing between concepts
  • Group vertically related concepts
  • Declare local variables close to their use
  • Declare instance variable at the top
  • Caller function should be close to callee and should come first

As always dealing with smaller files is easier to apply this rule. As a general guideline, Uncle Bob suggest keeping them at about 200 lines, with a maximum of  500 LoC.

In the same vein he suggest to set a maximum of 100-120 characters for a line of code, while having most of it being less than 60.

indentation is really important to help legibility, so don’t collapse a scope even if it is possible.

 

if(value == null) return;
//...

While it is a valid statement the collapsed block might be overlooked by a reader wasting his time.

if(value == null)
    return;  //This is more readable
//...

Remember, trying to trick the 5 lines maximum rule for a function by collapsing statements only hinders the readability of your source file.

One last note, a project should look like it was written by a single individual, it should look consistent. Is not acceptable to have different parts of it have different coding styles. The team rules, so at the start gather your group and decided on which rules to follow, than follow them.

Clean Comments

One of the most interesting things I’ve found out about people arguing on the internet is related to code comments. Every time there’s an article about how to properly comment code it is followed by a heated discussion in the comment section. Two sides. One saying that comments are really, really important, and the other saying the exact opposite. Comments should be avoided when possibile.

What I’ve realized is that your stance on comments depends heavily on your experience, and it isn’t a linear progression from NO COMMENTS to PRO COMMENTS (or viceversa), it is more of a wave function. You usually start in the NO COMMENTS camp, when you are alone writing code for fun, then you go all the way to the PRO COMMENTS when you start dealing with other peoples code (or your old code) and spending hours trying to understand a function, and wish that someone had commented it. Then suddenly you go to the NO COMMENTS again when you have to deal with old lying comments that mislead you, and realize that developers won’t update the comments as the code changes. Then (at least this is where I’m now, who knows tommorow what I’ll think) you find some middle ground where you avoid comments where possibile and write them when you fail to write code that’s self documented.

That last phrase “self documented code” is used by people on the NO COMMENTS side, both experienced and not experienced programmers, so people on PRO COMMENTS think that when arguing about comments they are dealing with noobies, and maybe they don’t even know that exist experts whom are one the NO COMMENTS camp. These guys on the other hand have a really hard time breaking that first impression and everything they say just lumps them more no the noobie group.

There. Internet wars explained. Now let’s stop ranting and let’s see how to write good comments and what comments we should avoid.

First a  general guideline. I can’t recall where I read it but the best rule of thumb for comments I’ve found is this

Don’t write comments that say what the code does. Write comments that explain why.

I’ve seen comments like this, and I immediatly delete them

string x = start + end; //Concatenate start and end
int y = a + 1; //increment a

Those comments don’t help in any way. They just say in more words exactly what the code is alreday saying. They are noise, and the human brain is really good ad ignoring noise. I don’t want my brain ignoring comments, because comment can contain really important information, so when I see a comment that is noise I delete it.

What could have been useful in the previous snippet was a comment explaining why a was being incremented. Right now I’m left wondering, and if the code is written properly I should be able to understand while reading  more of it. But sometimes it isn’t possible to express intent with code alone, in these cases a comment can help clear things up.

The why behing a piece of code can come in multiple forms, like:

  • Explaining intent
  • Warning of consequences
  • Justifiing something that without proper context could look wrong – like in the case of performance reasons

Another kind of good comments are documentation comments on public API, but only if they actually add relevant information that can’t be inferred from the signature of a method.

The worst kind of comment are out of date comments, because it spreads lies. Obviously no programmer has ever written an old comments. Comments become obsolete after they where written, because they tend to not being updated.

Comments that you should avoid are:

  • Comments that leave too much to interpretation – if you are taking the time to add a comment, make sure it is clear
  • Redundat – like in the example before
  • Mandated comments – policies shouldn’t require programmers to write comments, because they will write lots of redundant comments. Policies should require clean code
  • Comments that try to do the job of a version control system. Like tracking who edited what, when and why. Use a VCS

A special place in hell should be reserved to commented out code. Be rutheless and delete it as soon as you see it.

It’s important to remember to never use a comment when a name would do a better job, like in the following examples

DateTime date; //last modified
DateTime lastModified;

void functionName(){
   //Do this
   ...
   //Do that
   ...
   //Do something else
   ...
}
void functionName(){
   DoThis();
   DoThat();
   DoSomethingElse();
}

 

Clean Functions – Clean Code

Apollo 8 liftoff

Now let’s take a look at rules that give you clean functions.

First off, your functions should be smaller than you think. Many people recommend functions of 20 lines, probably referring to the old times when that was how much code you could see on a computer screen, but Uncle Bob recommends a maximum of 5 lines, and I whole heartily agree with him. I hear you! 5 lines sounds like too little, sounds like you won’t be able to write a functions that does anything useful.

Well you should give it a try. The trick to building useful programs with small functions is to use many of them and composing your logic with well named, easy to understand, nice, small little blocks.

In a next post will do a nice refactoring screencasts that will guide you in breaking a big bad 400 lines function in a swarm of small functions to drive home this message. For now just remember, 5 is plenty, 5 is your MAXIMUM your functions should be smaller. And no, those terrible one liners when you declare an object, instantiate it, call 20 methods and then return another object don’t count as “one line”, those count as a whole module and you should stop using them. Remember that clean code means code that is easy to understand, that guides you. We have rules like a maximum line count to help us reach a goal, being clever with the interpretation of a rule won’t help.

Functions should do one thing only, this is quite easier when your functions are small, and importantly they should work on only one level of abstraction. Mixing and matching here is terrible for the reader that first approaches a function. They will have a hard time because they are trying to figure out what a function does, but they will be bombarded by details from deeper levels that they currently don’t care about.

An example? Well, I’ve seen (and written) functions that send scheduled emails that have mixed concepts like:

  • When should the emails be sent
  • Who should receive this emails
  • What data you need to send
  • How to get the data you need to build this emails
  • Building the emails themself, down to the html tags

So not only this function was doing many different things, it was also operating on many different abstraction layers like sharepoint CAML queries, HTML syntax, string concatenation. All those things should be done by different modules of a program, not by the same monster function.

We have already talked about naming things, same rules apply when naming functions. The names should be meaningful, descriptive. Don’t be afraid of using long names, specially for local functions. Long names are better than short names if the intent of the function is better explained, and your IDE will autocomplete them, so you won’t break your fingers while typing them.

Arguments to function make the function harder to understand. They usually are necessary, but not always.

logger.Log(message); //Is way easier to understand than
logger.Log(message, Level.Info, "http://localhost", false);

If you need to pass many arguments probably you can pass some of them to the constructor of the class. Try to use the least number of arguments possible, and remember:

enum ArgumentSize
{
    public const int Best = 0;
    public const int MaxValue = 3;
}

Don’t pass a boolean. That screams that the function is doing multiple things! Write two functions, one for the true case and one for the false. Give them a proper name so in the future you won’t be wondering what does calling the method Log() with a false at the end actually does.

If you need to pass multiple related arguments to a function, it’s best to group them together in a new object. The classic example is this

public void DrawLine(int x1, int y1, int x2 int y2){}
public void DrawLine(Point start, Point end){}

//Which is easier to understand?
DrawLine(10, 1, 15, 2);
//Or
var start = new Point(10, 1);
var end = new Point(15, 2);
DrawLine(start, end);

This also adheres better to the rule about working on the same level of abstraction. A function that draws lines should be dealing with points, not with integers.

You should avoid hidden side effects. This happens when a function does unexpected changes to the state of the programs. It can be modifying an instance variable or worse a global. It can also be modifying the parameters it received. This should be avoided when possible and if impossible, the name should convey this side effect so you remove the “hidden” part of the equation. That way at least the developer won’t be surprised.

A kind of side effect is an output parameter. That means a parameter that is passed to a function with the intention of returning its result in it. It can be quite hard to realize that the result of a function is in one of its parameters, so it is better to avoid this situation.

A nice convention to adopt is the Command Query Separation (CQS), this states that a function should either be a Query or it should be a Command. A query returns a value and has no side effect. A command is the opposite, it has side effects and returns void. There’s a lot to say about CQS so we will do it in a future blog post. In the mean time just remember that adhering to CQS makes your functions a lot easier to understand.

This has been said before but is worth repeating don’t return error codes, use exceptions. It makes your code easier to follow and it’s harder to accidentally ignore an error, and spend days debugging it.

Stop writing legacy Code – What’s Clean Code

Clean and dirty

If you want to stop writing legacy code we have seen the importance of doing TDD, because it let’s you refactor your code and improve it. Well, actually refactoring lets you change your code, modify it. To actually improve it you need to change it in a positive way, with a better structure. You need to know how to improve it. That’s why in the following blog posts will talk about some guidelines to clean our code.

The father of Clean Code is Robert C. Martin (or Uncle Bob), he has written a number of great books that I suggest you read. A great deal of what will talk about comes straight from the book Clean Code: A Handbook of Agile Software Craftsmanship that in my opinion is the bible  in this matter. Another important book is Code Complete: A Practical Handbook of Software Construction by Steve McConnell.

We will first have a look at names and how to choose them in a way guides us through the code, than we’ll talk about functions how big (or small) they should be, then how to (not) write comments and some guidelines for your classes. Finally after we have clean production code we will explore the importance of having also clean test.

But what does it mean to write Clean Code. It means writing code that is easy to reason about. When you first approach it, or when you navigate in it you should feel like you know it. The names should guide you, and every function should turn up to be pretty much what you expected.

Working on clean code is a great experience and since you are the one writing code… why not write it clean?

Clean Names – one of the hard things

Clean names are hard

There are only two hard things in Computer Science: cache invalidation and naming things.

Phil Karlton

Today we are talking about the second of the hard things, luckily this is the one you can keep on improving with ease. I’ll summarize the most important rules you should follow and in future posts we can expand on them.

If there is one thing that you have to keep in mind is: naming is Hard but names are also flexible, if you pick a name and later on find a better one, change it! It will improve your understanding of the software next time you read that file. This flexibility isn’t an excuse to pick a terrible name right now (since you can change it later). No! You must think hard about the names you choose, but also you must change that name if later on you think of a better one.

Use your IDE tools to rename your code, it’s just a quick shortcut away and it won’t forget to change a reference in another part of your project.

Rules

First of all use meaningful names. Names that explain what that object is or does as much as possible.

Date d;            //doesn't mean anything
Date date;         //it's just repetition
Date lastModified; //This is something useful

The problem with meaningful names is that they are so useful that you start to relay on them (ok that’s not the problem). The real problem happens when you start finding names that look meaningful but actually are a bit of disinformation. What would happen if the requirements change and the variable lastModified now contains the date when a user last read a file? Well, now that name is a lie and it should be changed as soon as possible.

Another possible problem occurs when you use a name that can have different meanings in different contexts, or if you keep referring to the same concept with different words.

Part of what makes naming hard is that sometimes you want to reuse a name but you can’t because the compiler won’t let you. Many times this is actually a problem with the size of your function/classes, if you split a function in smaller ones its local variables are isolated from each other and you can reuse a name (when it’s the most meaningful name to use). Other times novice programmer will try to fix this by changing the names in ways that simply add noise to your programs. The compiler won’t complain but your fellow programmers should.

Check the following two functions

function TransferMoney(int id1, int id2){
    var customer1 = GetCustomer(id1);
    var customer2 = GetCustomer(id2);
    TransferMoney(customer1,customer2);
}
function TransferMoney(int receiverId, int senderId){
    var receiver = GetCustomer(receiverId);
    var sender = GetCustomer(senderId);
    TransferMoney(receiver, sender);
}

Say what you want but in the second one is way clearer whose sending money to whom, and that’s in a 3 line function. How many functions do you have that have way more than 3 lines?

Names should be words you can pronounce. It’s quite common to talk about a pice of code with another programmer, and if you have to spell out loud or you sound stupid saying the name of a variable, that name is hindering your communication. A few abbreviations are acceptable, but you need to keep in mind that it has to be easy to pronounce the names. This rule also excludes most of the encodings (hungarian notation I’m looking at you). In a modern IDE encodings have no place to stay, let your tools gather for you the information about types and access and keep your names simple. No need to burden your mind with this kind of information when your computer can do a better job at it.

Good names are also searchable and that helps you navigate your code and find what you are looking for. In this regard, is quite useful to give a name to magic numbers. If you have to find where you used a variable named weekDays is quite easy. Finding the right instance of the number 5 is a lot harder.

Programmers like sometimes to make inside jokes when naming functions like Apocalypse() instead of a simple DeleteAll(). While this might be fun it isn’t professional and makes life harder to your fellow programmers so no funny names.

Choose names familiar to other programmers (list, visitor, queue) and from the problem domain. Whenever is possible is best to use the same language that your customer uses, that saves you the trouble to keep translating from one language to another.

And lastly: don’t prefix all your names with your project name or an abbreviation. Adding the same prefix everywhere doesn’t add any meaningful information and hinders your ability to work. This is known as smurf naming convention and you should look up its definition.

If you have ever SPWorked on a SharePoint SPSolution there’s no SPNeed to reiterate this SPPoint.