Programming best practices; you should always follow them
As you know, I'm a .Net developper and I love to develop softwares and websites. I love when I do something all by myself because everything is done the same way and the code is clear and readable. When I have to review other's code or add something in an existing project I always have difficulties with it because each programmer has it's own way to code, and a lot of programmers don't know how to code. Sometimes I look at some code and think "How can someone thinks about the problem and solves this that way. How can he read his code to add something in it... ?". I think it's time to give you some general rules you have to follow whatever your programming language is.
Never comment you code, except methods signatures
This is a simple and clear rule. You code HAS to be self-explaining. Name you variables well, avoid abreviations, avoid generic names, etc... Let's take a look at this piece of code :
Bad code
public WeatherForcastData Func()
{
/* Declare the username used to connect to the webservice */ string name =
"WebMasterSam";
/* Declare the variable that will contain the password
Here we put it in clear text but normaly we don't */ string pass =
"Pass@word1";
/* Instanciate the webservice so we can call the WebMethod
to get the weather */ WebService service =
new WebService();
/* Assign the credentials to the service so we don't get
an error */
service.Credentials = new NetwrokCredentials(name, pass);
/* Call the WebMethod to retrieve weather forcast */
return service.GetWeatherForcastData("CITY");
}
Good code
/// <summary>
/// Get the weather forcast for a specific city by calling the WeatherForcast WebService.
/// </summary>
/// <remarks>We have to set the credentials to call the WebService because it's not a public free service.</remarks>
public WeatherForcastData GetWeatherForcast()
{
string userName = "WebMasterSam";
string password = "Pass@word1";
string cityName = "CITY";
WebService service = new WebService();
service.Credentials = new NetwrokCredentials(userName, password);
return service.GetWeatherForcastData(cityName);
}
Adding a lot of comments in the code makes it really heavy and hard to read. Keep your comments in the method signature. Method signature is used by intellisense, so when you call a method you see the information in a yellow pop-up.
In functions, always have a single exit point
This is sometimes simpler to add "return" statements everywhere in the code if you, for example, only validate data and want to return False if the validation fails. When you do this, and someones passes after you to add something, he will not necessarily see all your return statements and he can miss one. It might not appear in his little tests but appear later when you send your application for approbation by the client, or even worse, in production.
When you have a function, your last line of code should be the return statement. Take a look a this piece of code :
Bad code
public bool ValidateInput(
string param1,
string param2,
int param3,
int param4)
{
if (param1.Length == 0)
return false;
else {
if (param1.Length > 100)
return false;
if (param2 == "NONE")
return false;
else
{
if (param3 < 0 || param3 > 100)
return false;
if (param4 == 0)
return false;
}
}
return true;
}
Good code
public bool ValidateInput(
string param1,
string param2,
int param3,
int param4)
{
bool isInputValid =
true;
isInputValid = isInputValid && (param1.Length > 0 && param1.Length < 100);
isInputValid = isInputValid && (param2 != "NONE");
isInputValid = isInputValid && (param3 >= 0 && param3 <= 100);
isInputValid = isInputValid && (param4 != 0);
return isInputValid;
}
I rewrote the method like this but if you don't like the way I did it you can do it another way. The important thing to remember is that you need only one return statement at the end that returns a well-named variable that tells exactly what it returns.
Keep your methods significant (split them as needed)
Many programmers have the bad habit to create big methods and put all their code in it. Even worse, some of them put the code directly on the event handlers. Normaly you should keep your methods no longer than 50 lines of code.
Each method you write should have only one signification. For example, if you have a routine that will contact a webservice, add data to a database, create a text file and write to the event viewer, please don't write all your code in the same method. Split your code into many methods.
Bad code
public void MegaMethod()
{
/*
add code to contact webservice
add code to add data to the database
add code to create a text file
add code to write to the event viewer
*/
}
Good code
public void MegaMethod()
{
ContactWebService();
AddToDataBase();
CreateTextFile();
WriteToEventViewer();
}
In addition to this, when people all put in the same method, they tend to add a lot of comments to help understand when the method does. As stated in the first point, this is bad. By spliting your code into different methods, you can add comments in the methods signatures, not in the code. When you split into many methods, name them significantly because you have to keep your code self-explaining.
Don't put all in member variables, pass values as method parameters
Normaly, a function should receive parameters, treat them and output a value. Keep it this way. When you have a lot of methods, some people create a lot of member variables, affect them and use them in their different methods. It's a bad practice because you don't what your functions need to do their job.
Always have the same naming convention
It's a very simple and easy thing to follow, but how many people absolutely don't care about this. When you write constants, always write them the same way. When you write methods, always write them the same way... just look at this :
Bad code
private string member1 = "";
private string Member_2 = "";
private int iMember3 = 0;
private const string Const1 = "";
private const string CONST2 = "";
public void methodName1() { }
public void Method_Name_2() { }
Good code
private string _member1 = "";
private string _member2 = "";
private int _member3 = 0;
private const string CONST_1 = "";
private const string CONST_2 = "";
public void MethodName1() { }
public void MethodName2() { }
It looks obvious but I see bad naming like this every day. If you don't like screaming caps for constants, fine, you can use camel casing, but always use camel casing for your constants. In fact, when you start a project, establish the naming convention and always stick to it strictly.
Think about it before coding it
Yes, think about it. You will save time and effort. How many programmers are too pressed, too stressed that they can't wait a minute to start coding ? a lot ! This is more efficient to sit and think about what you have to code for about 10 minutes and then start to code for about an hour than immediatly start to code and patch your code the rest of the day.
When you think about how you will code, you find code than can be reused and you can find possible dead-ends.
In the end...
All of this is what I see everyday when I check other's code. I don't have to tell you I check everybody's code even if it's not officialy part of my job ! If you follow those logical and simple rules, you will see a big difference when you have to read back your code to add something and you will find it easy to read other's code.
Remember... always KISS (Keep It Simple and Stupid) !