| A | B | C | D | E | F | G | H | ||
|---|---|---|---|---|---|---|---|---|---|
1 | See http://gerleim.wordpress.com/2014/07/28/net-c-code-review-checklist/ | Infomatix Ltd. | ProjectA | ProjectB | |||||
2 | Area | Rule short description | Rule Code | Type | What | Description/Comment | Result | Result | |
3 | Environment setup | ||||||||
4 | Solution and project names | CR SnP n | The name of the solution and the projects are appropriate | y | y | ||||
5 | Assembly and namespace names | CR AnNS n | y | y | |||||
6 | AssemblyInfo | CR AI | P LP | AssemblyInfo.cs contains appropriate information | n | n | |||
7 | Semantics | ||||||||
8 | StyleCop rules | CR SC | All “normal” StyleCop requirement should met. | y | y | ||||
9 | Clean Code / Structure | ||||||||
10 | Flow control | CR Str flow | “Reasonable” structuring of flow control structures. | Use as few nesting as possible. | y | y | |||
11 | Everything is in its place | CR Str place | Is a piece of code in its place? | BL in UI code. Executing logic in a Controller. Logic in data access etc. | y | n | |||
12 | Top Down structure | CR Str td | LP | x | x | ||||
13 | Naming - namespaces | CR N NS | LP | Appropriate naming of namespaces | y | y | |||
14 | Naming - classes | CR N C | Appropriate naming of classes | y | y | ||||
15 | Naming - methods | CR N M | Appropriate naming of methods | y | y | ||||
16 | Classes - class structure and responsibility | CR SOL | H | SOL from SOLID | y | m | |||
17 | Mehtods - lenght | CR ML | Methods should have a maximum of 30-40 lines of code. | y | y | ||||
18 | Setters contains minimal logic | CR SNL | y | y | |||||
19 | General | ||||||||
20 | Unit Testing | CR UT | P | Write developer test cases and perform unit testing to make sure that basic level of testing is done. | A separate rule set in itself. Individual requirements of unit testing is per project per leader (expected areas, granularity, and code coverage). | ? | ? | ||
21 | Constants | CR const | Magic numbers and inline strings should be moved to const (or static read only) variables. | n | n | ||||
22 | Disposing of Unmanaged Resources | CR DR | Disposing of Unmanaged Resources like File I/O, Network resources, etc. They have to be disposed of once their usage is completed. Use usings blocks if you want to automatically handle the disposing of objects once they are out of scope. | x | x | ||||
23 | Multi-threading | CR MT | Proper multi-threading techniques. | In multi threading code, use proper locking. The lock keyword should use unique lock objects. Do not use not-synchronized collections, or ensure the proper locking. Note: Testing MT is hard | x | x | |||
24 | String concatenation | CR StrC | Use string.Concat() or a StringBuilder instead of string if multiple concatenations are required. Use a StringBuilder when large or complicated string concatenation is done. | Reasoning: Concatenation creates new instances of string. SB will execute (lot) faster, and saves heap memory. Note: One line concatenation is optimized by the compiler. string.Format()and string.Append()are also concatenate. | y | y | |||
25 | Dead code | CR DC | LP | No unused, never called, commented code should stay. | Reasoning: Let’s source control handle these. | y | y | ||
26 | TODOs | CR TD | LP | Should not contain a lot of TODOs. | "Living" code (code before a longer dev timeline) is exception. | m | m | ||
27 | Argument checks | Checking method parameters for null, empty and invalid values. Only in public interfaces | In public APIs. Internal methods should not check parameters. Ensure that they give proper values, when called. | m | m | ||||
28 | Error handling | ||||||||
29 | Exception Handling | CR Eh | Proper implementation of Exception Handling (try/catch and finally blocks) | y | y | ||||
30 | Logging | CR ehl | P | Errors/exceptions are logged | ? | n | |||
31 | Error, displaying contents | CR S ErrD | Do not display internal information to clients. | ? | ? | ||||
32 | Exception type | CR ET | Wrong/not the right type of exception is used/thrown | n | n | ||||
33 | Security | ||||||||
34 | Logging, storing contents | CR S ErrL | Do not log sensitive data. (Technical usernames, like db access user, or passwords etc.) | na | na | ||||
35 | Information on User tokens | CR UM existing | do not give information to users if a user is existing / user token is used. (With the exception of already taken unique tokens). | Example: password reminder function should not indicate whether an email address is already used in a system. | na | na | |||
36 | Password storing | CR UM pswd | do not store password in clear text. Use latest techniques of password storing (hash and random salt). | na | na | ||||
37 | Optimization | ||||||||
38 | Optimization general | CR Opt | Avoid premature optimization. Do not micro-optimize. Do not optimize without measurement. | Note: remember, aside extreme cases DB access and out-of-process calls will execute magnitudes slower than anything happening in application logic. | na | na | |||
39 | Comments | ||||||||
40 | Comments (XML comment) | CR C XML | All public members should have valid XML comment. | Note: the examination of existence is part of SC. Additionally, examine if the content and wording is correct. Beare of automatically generated but meaningless or invalid comments. Supression is allowed. | n | n | |||
41 | Comments in code | CR C Code | Only in extreme and valid cases. One case is 3rd party (or not possible to refactor) component usage with not CC interfaces. | n | n | ||||
42 | Comment, missing | CR C M | Indicate if a comment is missing. | Dev at least should inspect the validity of the request. | n | y | |||
43 | Other | ||||||||
44 | Other | CR X | Indicates any other CR item. | Example: A class with only one method, which only wraps something. The CR code (and note added) questions if it is neccessary. | n | n |