In computer programming, code smell is any symptom in the source code of a program that possibly indicates a deeper problem. We are going to look at some of them here.
1. Catch me if you can handle it: Whenever you are doing an exception handling in your code, do not just use the throw keyword to throw the exception to your parent method but instead, throw with the current exception so that the current stack trace information is also sent and you do not lose original exception information.
Do not use
catch (Exception ex)
{
throw;
}
instead, use
catch (Exception ex)
{
throw ex;
}
2. InPlace Replace: We have used the string functions so many times, but one among them is the Replace function, which returns back the replaced string instead of in-place replacing the string.
For example
string s = "this is my string";
s.Replace("this", "that"); // This will not replace the word
s = s.Replace("this", "that"); // This will replace the word
3. Number Magix: There are instances where we need to use hardcoded numbers or strings, which are also known as "magic numbers/strings" in your code. Try to avoid scattering them in your code, but instead, create enums or store them in the config files.
// bad code
if (day == 3) { ...
} else if (day == 4) {
...
}
// Use Enumerations wherever possible, so the meaning and not the number is displayed in the code. It helps debugging and readability for you and others who may maintain your code:
// good code
if (day == MyEnum.ThirdDay) {
...
} else if (mode == MyEnum.FourthDay) {
...
}
4. Builder by your side: One should Whenever trying to concatenate a lot of string, you should not try to use the string object since that creates a lot of footprints in your memory; instead use StringBuilder.
// bad code
string s = string.Empty;
for (int i = 0; i < 100; i++) {
s = s + "This is number " + i.ToString();
}
// good code
StringBuilder mBuilder = new StringBuilder();
for (int i = 0; i < 100; i++) {
mBuilder.Append("This is number " + i.ToString());
}
Please also look into String.Concat() if you want to have a contactless number of strings, that would be ideal.
5. Wire/UnWire: Whenever you are writing code to Wire an event, also remember to unwire that event, as that might give you unexpected results. I have seen comments that discuss how unwiring is taken care of by the garbage collector, but that is not the case. So, one should unhook event handlers appropriately after wiring them.
6. What to catch finally: Many times, we have seen programmers use the try-catch block, but they do not write anything in the catch block as they just want to pass the code. This may hold true in a case which I will discuss below, but ideally, one should not do the following:
// bad code
try
{
// my code may have errors here
}
catch (Exception ex)
{
// I do nothing with this error, just gulp it.
}
You should do some logging of the exception or throw the exception to the parent caller. The case where this could be a possibility is below, but again, there are ways to overcome this scenario, and if you happen to fall into one of these, please leave a comment on what you did.
try
{
// my code may contain an error.
}
catch (Exception ex)
{
try
{
LogMyException(ex); // this might also throw an error
}
catch (Exception ex)
{
// what to do here now?
}
}
Also, I have seen coders writing catch blocks always use generic exception objects that are not specific to the expected error. One should first try to catch the specific error and also keep the generic as a fallback like below.
catch (OutOfMemoryException outmemex)
{
// do something to release the memory, logging
}
catch (Exception ex)
{
// I don't know why an error has occurred, log it.
}
How can one not use the finally block, which guarantees the execution of the piece in spite of an exception? Please do not forget to add the finally block to release objects and connections used in your code.
7. Warn ok please: I am sure most of the coders here don't even bother about Compiler Warnings they receive when they are compiling their solutions, it is also important for you to look into the compiler warnings that are showing up.
8. Initialization: We have seen in our code that many times, we do unnecessary initialization of objects like the one below.
DataTable mytable = new DataTable(); // Initialization is not necessary
mytable = GetDataTable();
// One should instead use
DataTable mytable = GetDataTable();
9. Get, Set, Tools: Use more productivity tools and add-ins that make your life easier while coding one of them.
http://visualstudiogallery.msdn.microsoft.com/d0d33361-18e2-46c0-8ff2-4adea1e34fef
You may find this link very helpful.
http://visualstudiogallery.msdn.microsoft.com/site/search?f%5B0%5D.Type=RootCategory&f%5B0%5D.Value=tools
10. Are you NULL: So many times, we get a null reference exception, and that is probably because we always assume that there will be no null data, but that is always not true. One should always check for null before they use the object just to avoid these situations and have an alternate route to your code.
mType mobj = x as mType;
if (mobj != null)
{
// more code here
}
11. Follow standards and conventions while coding. In not doing so, you are taking a lot of others' time in your team to understand the code by making it hard to read and maybe re-use the code.
12. There is no need to call GC.Collect() in your code. The .NET GC is always there to collect your objects in most cases. It is a rare case when we need to interfere with the Garbage Collector's work.
13. String the String: I have seen many coders doing this, just to give you an example.
Request.QueryString["myvariable"]; // there is no need for ToString here
Request.QueryString returns a string, so one should not call.ToString() here.
14. Comment: Comment and document your code often, as you might also not remember that a complex piece or a variable++ had solved some problems in the past. So, not commenting on your code will create hard-to-read code for complex blocks for both you and your peers. Properly commented code is your friend.
15. Save the world: Stop thinking of yourself as a Hero, so when you are stuck with your code, just ask for help. It will not only help you solve your problem but also give a second eye to your issue and might come up with a more efficient way to solve the issue.
16. Avoid writing duplicate code. Just put that logging code in a utility module and keep calling from your code.
17. Do not be tempted to write long methods. What I mean here is method code, which runs in thousands of lines.
18. Do not have too many parameters in your methods; be reasonable, not confuse the caller of the method, and have the method readable.
19. Test, Test, Test: This will eliminate a lot of your bugs/defects and make your code more robust. This will increase the quality of your code and, in turn, your product reputation.
I hope these points were helpful to you and made your code smell better!