VN:R_U [1.6.3_896]
Rating: 4.5/5 (2 votes cast)
Good code comments are great. Useless ones are the root of all evil! Stop with the useless and redundant comments.
private void CreateTheProcessingThread()
{
// Create the processing thread
new Thread(ProcessingThread).Start();
} // End of method CreateTheProcessingThread()
Really?! I couldn’t really tell what was going on there… all of those ridiculous comments were clouding what was really going on. Making the comment the actual name of the method and adding spaces is just asinine!
And do you really need an entire method to make one call? It’s okay if you do because you might eventually change the implementation of that method… but please just do it like this instead:
private void CreateTheProcessingThread()
{
new Thread(ProcessingThread).Start();
}
VN:R_U [1.6.3_896]
Rating: 4.5/5 (2 votes cast)
VN:R_U [1.6.3_896]
Rating: 5.0/5 (1 vote cast)
Sometimes, programmers literally translate exactly what they are thinking directly into code. This is especially true for IF/ELSE cases. So, if you see this:
if (someCondition)
return DoSomething1();
else
return DoSomething2();
…the ‘else’ statement is redundant because if the ‘if’ condition was true the method would return right there. Instead, write that same code like this:
if (someCondition)
return DoSomething1();
return DoSomething2();
VN:R_U [1.6.3_896]
Rating: 5.0/5 (1 vote cast)
VN:R_U [1.6.3_896]
Rating: 5.0/5 (2 votes cast)
When you see code like this:
private void ThisCodeReallySux(string action_)
{
if (action_ == "Action1")
DoAction1();
else if (action_ == "Action2")
DoAction2();
else if (action_ == "Action3")
DoAction3();
else if (action_ == "Action4")
DoAction4();
else if (action_ == "Action5")
DoAction5();
else if (action_ == "Action6")
DoAction6();
else if (action_ == "Action7")
DoAction7();
else if (action_ == "Action8")
DoAction8();
else if (action_ == "Action9")
DoAction9();
else if (action_ == "Action10")
DoAction10();
}
Replace it with a “delegate dictionary” and a method that loads the dictionary when an instance of the class is instantiated.
private readonly Dictionary _actionMap
= new Dictionary();
// LoadActionMap should be called from the class' constructor.
private void LoadActionMap()
{
_actionMap["Action1"] = DoAction1;
_actionMap["Action2"] = DoAction2;
_actionMap["Action3"] = DoAction3;
_actionMap["Action4"] = DoAction4;
_actionMap["Action5"] = DoAction5;
_actionMap["Action6"] = DoAction6;
_actionMap["Action7"] = DoAction7;
_actionMap["Action8"] = DoAction8;
_actionMap["Action9"] = DoAction9;
_actionMap["Action10"] = DoAction10;
}
Instead of having all the IF/ELSE code, you simply have two lines of code… no matter how many “actions” you have. Of course, in this example, there is no logging or reporting of the case when an unknown action arises… but this is example code!
private void ThisCodeIsMuchBetter(string action_)
{
if (_actionMap.ContainsKey(action_))
_actionMap[action_]();
}
VN:R_U [1.6.3_896]
Rating: 5.0/5 (2 votes cast)