My programming tidbits – Clear Code

 

Hi All,

I know I didn’t write a full post for quite a while. I was quite busy with work and other personal issues but I assure you that I am working on something interesting for the next full post. Meanwhile, I will start a series of short posts where I describe my programming tidbits. I will try to write something different than the regular “Write tests! Don’t write long methods! Don’t use the mouse!” that you can read anywhere online. My number 1 tip is this (and if you are going to skip all the other tips, please still consider this one):

Write code that is clear for all the team members.

I will start with an example. I was sent a piece of code for code review which is similar to this one:

11. public string Foo()
12. {
13.   string fileName = Path.GetTempFileName();
14.   Func<string> func = () => { return Path.Combine(Directory.GetCurrentDirectory(), fileName); };
15.  
16.   string fullFileName = func();
17.   while (!File.Exists(fullFileName))
18.   {
19.     fileName = Path.GetTempFileName();
20.     fullFileName = func();
21.   }
22.  
23.   return fullFileName;
24. }

This code generates an inexistent temporary file name in the current directory. I kindly asked to change the code to the following:

26. public string Foo2()
27. {
28.   string fullFileName;
29.   do
30.   {
31.     fullFileName = Path.Combine(Directory.GetCurrentDirectory(), Path.GetTempFileName());
32.   }
33.   while (!File.Exists(fullFileName));
34.  
35.   return fullFileName;
36. }

After a short argument it was agreed that my code is slightly better in terms of readability (lets leave the performance considerations aside for this example). The main reason that initially this was not the code is because the engineer who wrote the code doesn’t like the do-while loop. To tell the truth, I don’t like do-while loop either. There is something not natural about its flow but in this case it saves a lambda expression, generation and allocation of a new class, and some readability.

When writing code please remember that not all your team members know everything you know. Some might not know lambda expressions well while others might not be familiar with the syntax of LINQ, some don’t like using “var” everywhere while others don’t want to clear unused “using”s because then they miss fundamental system namespaces by default. I am not saying you should not use these advanced framework features. I am saying you should not abuse these features (as in the above example). Think of the poor programmer that has to understand the entire logic of your algorithm and then he sees this method. He may not be best friends with lambdas but now he has to understand what this method does and why the hell did you not use a simple do-while loop instead.

My conclusion: Use advanced language features when they bring benefit to the code and not in order to make your code look cool.

 

Thanks for reading,

Boris.

 

P.S.

If the person who sent this code to me is reading this. Please don’t take this personally, this is just a good example of my tip.