Saturday 8 March 2014

C# code review checklist


    1. Are exceptions used to indicate error rather than returning status or error codes?
    2. Are all classes and public methods commented with .NET style comments?  Note that <summary> comments should discuss the "what" of public methods.  Discussion of "how" should be in <remarks> blocks or in-line with the code in question.
    3. Are method arguments validated and rejected with an exception if they are invalid?
    4. Are Debug.Asserts used to verify assumptions about the functioning of the code?  Comments like, "j will be positive" should be rewritten as Asserts. 
    5. Do classes that should not be instantiated have a private constructor?
    6. Are classes declared as value types only infrequently used as method parameters, returned from methods or stored in Collections?
    7. Are classes, methods and events that are specific to an assembly marked as internal?
    8. Are singletons that may be accessed by multiple threads instantiated correctly?  See the Enterprise Solution Patterns book, p. 263.
    9. Are methods that must be overriden by derived classes marked as abstract?
    10. Are classes that should not be overriden marked as sealed?
    11. Is "as" used for possibly incorrect downcasts? 
    12. Do classes override ToString instead of defining a Dump method for outputting the object's state?
    13. Are log messages sent to the logging component instead of Console?
    14. Are finally blocks used for code that must execute following a try? 
    15. Is foreach used in preference to the for(int i...) construct?
    16. Are properties used instead of implementing getter and setter methods?
    17. Are readonly variables used in preference to properties without setters?
    18. Is the override keyword used on all methods that are overriden by derived classes?
    19. Are interface classes used in preference to abstract classes?
    20. Is code written against an interface rather than an implementing class?
    21. Do all objects that represent "real-world" or expensive resources implement the IDisposable pattern?
    22. Are all objects that implement IDisposable instantiated in a using block?
    23. Is the lock keyword used in preference to the Monitor.Enter construct?
    24. Are threads awakened from wait states by events or the Pulse construct, rather than "active" waiting such as Sleep()?
    25. If equals is overridden, is it done correctly?  The rules for overriding equals are complex, see Richter p153-160 for details.
    26. If == and != are overridden, so they redirect to Equals?
    27. Do all objects that override Equals also provide an overloaded version of GetHashCode that provides the same semantics as Equals?  Note that overrides to GetHashCode should take advantage of the object's member variables, and must return an unchanging hash code.
    28. Do all exception classes have a constructor that takes a string and and another constructor that takes a string and an exception?
    29. Do all exception classes derive from the base Matrix exceptions and fit correctly into the exception hierarchy?
    30. Are all classes that will be marshaled or remoted marked with the Serializable attribute?
    31. Do all classes marked with the Serializable attribute have a default constructor?  This includes Exception and EventArgs classes.
    32. Do all classes that explicitly implement ISerializable provide both the required GetObjectData and the implied constructor that takes a SerializationInfo and a StreamingContext?
    33. When doing floating point calculations, are all constants doubles rather than integers?
    34. Do all delegates have a void return type and avoid using output or ref parameters?
    35. Do all delegates send the sender (publisher) as the first argument?  This allows the subscriber to tell which publisher fired the event. 
    36. Are all members of derived EventArg classes read-only?  This prevents one subscriber from modifying the EventArgs, which would affect the other subscribers.
    37. Are delegates published as events?  This prevents the subscribers from firing the event, see Lowy, p. 102 for details.
    38. Is common setup and teardown nUnit code isolated in Setup and Teardown methods that are marked with the appropriate attribute?
    39. Do negative nUnit tests use the ExpectedException attribute to indicate that an exception must be thrown?
     
  1.  
  2. Ensure that there shouldn't be any project warnings.
  3. It will be much better if Code Analysis is performed on a project (with all Microsoft Rules enabled) and then remove the warnings.
  4. All unused usings need to be removed. Code cleanup for unnecessary code is always a good practice.

    Refer to: http://msdn.microsoft.com/en-us/magazine/ee335722.aspx.
     
  5. A "null" check needs to be performed wherever applicable to avoid the Null Reference Exception at runtime.
  6. Naming conventions are to be followed always. Generally for variables/parameters, follow Camel casing and for method names and class names, follow Pascal casing.

    Refer to: http://msdn.microsoft.com/en-us/library/ms229043.aspx.
     
  7. Ensure that you are aware of SOLID principles.

    Definition from Wikipedia: In computer programming, SOLID (Single responsibility, Open-closed, Liskov substitution, Interface segregation and Dependency inversion) is a mnemonic acronym introduced by Michael Feathers for the "first five principles" identified by Robert C. Martin in the early 2000s that stands for five basic principles of object-oriented programming and design. The principles when applied together intend to make it more likely that a programmer will create a system that is easy to maintain and extend over time. The principles of SOLID are guidelines that can be applied while working on software to remove code smells by causing the programmer to refactor the software's source code until it is both legible and extensible. It is typically used with test-driven development, and is part of an overall strategy of agile and adaptive programming.

    Refer to: http://en.wikipedia.org/wiki/SOLID_(object-oriented_design)
     
  8. Code Reusability: Extract a method if the same piece of code is being used more than once or you expect it to be used in the future. Make some generic methods for repetitive tasks and put them in a related class so that other developers start using them once you intimate them. Develop user controls for common functionality so that they can be reused across the project.

    Refer to:

    http://msdn.microsoft.com/en-us/library/office/aa140806(v=office.10).aspx
    o  http://blogs.msdn.com/b/frice/archive/2004/06/11/153709.aspx
     
  9. Code Consistency: Let's say that an Int32 type is coded as an int and a String type is coded as a string, then they should be coded in that same fashion across the application. But not like sometimes an int and sometimes as an Int32.
  10. Code Readability: Should be maintained so that other developers easily understand your code.

    Refer to: http://msdn.microsoft.com/en-IN/library/aa291591(v=vs.100).aspx
     
  11. Disposing of Unmanaged Resources like File I/O, Network resources, etcetera. They must be disposed of once their usage is completed. Use using blocks for unmanaged code, if you want to automatically handle the disposing of objects once they are out of scope.

    Refer to: http://msdn.microsoft.com/en-us/library/498928w2.aspx
     
  12. Proper implementation of Exception Handling (try/catch and finally blocks) and logging of exceptions.

    Refer to: http://msdn.microsoft.com/en-us/library/vstudio/ms229005(v=vs.100).aspx
     
  13. Ensuring that methods have fewer lines of code. Not more than 30 to 40 lines.
  14. Timely check-in/check-out of files/pages at source control (like TFS).

    Refer to: http://www.codeproject.com/Tips/593014/Steps-Check-in-Check-Out-Mechanism-for-TFS-To-avoi
     
  15. Peer code reviews. Swap your code files/pages with your colleagues to perform internal code reviews.
  16. Unit Testing. Write developer test cases and perform unit testing to ensure that a basic level of testing is done before it goes to QA testing.

    Refer to: http://msdn.microsoft.com/en-us/magazine/cc163665.aspx
     
  17. Avoid nested for/foreach loops and nested if conditions as much as possible.
  18. Use anonymous types if code is going to be used only once.

    Refer to: http://msdn.microsoft.com/en-us/library/vstudio/bb397696.aspx
     
  19. Try using LINQ queries and Lambda expressions to improve readability.

    Refer to: http://msdn.microsoft.com/en-us/library/bb308959.aspx 
     
  20. Proper usage of var, object, and dynamic keywords. They have some similarities that cause confusion for developers or don't know much about them and hence they use them interchangeably, which shouldn't be the case.

    Refer to: http://blogs.msdn.com/b/csharpfaq/archive/2010/01/25/what-is-the-difference-between-dynamic-and-object-keywords.aspx
     
  21. Use access specifiers (private, public, protected, internal, protected internal) as per the scope need of a method, a class, or a variable. Let's say if a class is meant to be used only within the assembly, then it is enough to mark the class as internal only.

    Refer to: http://msdn.microsoft.com/en-us/library/kktasw36.aspx
     
  22. Use interfaces wherever needed to maintain decoupling. Some design patterns came into existence due to the usage of interfaces.

    Refer to: http://msdn.microsoft.com/en-IN/library/3b5b8ezk(v=vs.100).aspx
     
  23. Mark a class as sealed or static or abstract as per its usage and your requirements.

    Refer to: http://msdn.microsoft.com/en-us/library/ms173150(v=vs.100).aspx
     
  24. Use a Stringbuilder instead of string if multiple concatenations are required, to save heap memory.
  25. Check whether any unreachable code exists and modify the code if it exists.
  26. Write comments on top of all methods to describe their usage and expected input types and return type information.
  27. Use a tool like Silverlight Spy to check and manipulate rendered XAML in Runtime of a Silverlight application to improve productivity. This saves a lot of back & forth time between Design & Run views of the XAML.
  28. Use the fiddler tool to check the HTTP/network traffic and bandwidth information to trace the performance of web applications and services.
  29. Use the WCFTestClient.exe tool if you want to verify the service methods out of the Visual Studio or by attaching its process to Visual Studio for debugging purposes.
  30. Use constants and readonly wherever applicable.

    Refer to:
    o   http://msdn.microsoft.com/en-us/library/acdd6hb7(v=vs.100).aspx
    o   http://msdn.microsoft.com/en-us/library/e6w8fe1b(v=vs.100).aspx
     
  31. Avoid type casting and type conversions as much as possible; because it is a performance penalty.

    Refer to: http://msdn.microsoft.com/en-us/library/ms173105.aspx
     
  32. Override ToString (from Object class) method for the types that you want to provide with custom information.

    Refer to: http://msdn.microsoft.com/en-us/library/ms173154(v=vs.100).aspx
     
  33. Avoid direct copying/pasting of code from other sources. It is always recommended to hand write the code even though you are referring to the code from some sources. By this, you will get good practice of writing the code yourself and also you will understand the proper usage of that code; finally you will never forget it.
  34. Always make it a practice to read books/articles, upgrade and follow the Best Practices and Guidelines by industry experts like Microsoft experts and well-known authors like Martin Fowler, Kent Beck, Jeffrey Ritcher, Ward Cunningham, Scott Hanselman, Scott Guthrie, Donald E Knuth.
  35. Verify whether your code has any memory leakages. If yes, ensure that they have been fixed.

    Refer to: http://blogs.msdn.com/b/davidklinems/archive/2005/11/16/493580.aspx
     
  36. Try attending technical seminars by experts to be in touch with the latest software trends and technologies and best practices.
  37. Understand thoroughly OOP concepts and try implementing it in your code.
  38. Get to learn your project design and architecture to better understand the flow of your application as a whole.
  39. Take necessary steps to block and avoid any cross scripting attacks, SQL injection, and other security holes.
  40. Always encrypt (using good encryption algorithms) secret/sensitive information like passwords when saving to the database and in connection strings stored in web.config file(s) to avoid manipulation by unauthorized users.
  41. Avoid using the default keyword for the known types (primitive types) like int, decimal, bool, etcetera. It should usually be used with Generic types (T) since we may not be sure whether the type is a value type or reference type. Refer to: http://msdn.microsoft.com/en-us/library/xwth0h0d(v=vs.100).aspx

No comments:

Post a Comment

Angular Tutorial (Update to Angular 7)

As Angular 7 has just been released a few days ago. This tutorial is updated to show you how to create an Angular 7 project and the new fe...