Common issues found in code review
I am going over a code base that I haven't seen in a while, and I am familiarizing myself with it by doing a code review to see that I understand what the code is doing now.
I am going to post code samples of changes that I made, along with some explanations.
This code can be improved by introducing a guard clause, like this:
This reduce nesting and make the code easier to read in the long run (no nesting).
I hope you recognize the issue. The code is using reflection to do an operation that is already built into the CLR.
This is much better:
Of course, there is another issue here, why the hell do we have those if statement on type instead of pushing this into polymorphic behavior. No answer yet, I am current just doing blind code review.
Here is another issue, using List explicitly:
It is generally better to rely on the most abstract type that you can use:
This is a matter of style more than anything else, but it drives me crazy:
I much rather have this:
Note that I added braces for both clauses, because it also bother me if one has it and the other doesn't.
Another issue is hanging ifs:
Which we can rewrite as:
I think that this is enough for now...