Code Review template
 Share
The version of the browser you are using is no longer supported. Please upgrade to a supported browser.Dismiss

 
View only
 
 
ABCDEFGH
1
See http://gerleim.wordpress.com/2014/07/28/net-c-code-review-checklist/
Infomatix Ltd.ProjectAProjectB
2
AreaRule short descriptionRule CodeTypeWhatDescription/CommentResultResult
3
Environment setup
4
Solution and project namesCR SnP nThe name of the solution and the projects are appropriateyy
5
Assembly and namespace namesCR AnNS nyy
6
AssemblyInfoCR AIP LPAssemblyInfo.cs contains appropriate informationnn
7
Semantics
8
StyleCop rulesCR SCAll “normal” StyleCop requirement should met.yy
9
Clean Code / Structure
10
Flow controlCR Str flow“Reasonable” structuring of flow control structures.Use as few nesting as possible.yy
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.yn
12
Top Down structureCR Str tdLPxx
13
Naming - namespacesCR N NSLPAppropriate naming of namespacesyy
14
Naming - classesCR N CAppropriate naming of classesyy
15
Naming - methodsCR N MAppropriate naming of methodsyy
16
Classes - class structure and responsibilityCR SOLHSOL from SOLIDym
17
Mehtods - lenghtCR MLMethods should have a maximum of 30-40 lines of code.yy
18
Setters contains minimal logicCR SNLyy
19
General
20
Unit TestingCR UT PWrite 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
ConstantsCR constMagic numbers and inline strings should be moved to const (or static read only) variables.nn
22
Disposing of Unmanaged ResourcesCR DRDisposing 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.xx
23
Multi-threadingCR MTProper 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
xx
24
String concatenationCR StrCUse 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.
yy
25
Dead codeCR DCLPNo unused, never called, commented code should stay.Reasoning: Let’s source control handle these.yy
26
TODOsCR TDLPShould not contain a lot of TODOs."Living" code (code before a longer dev timeline) is exception.mm
27
Argument checksChecking 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.mm
28
Error handling
29
Exception HandlingCR EhProper implementation of Exception Handling (try/catch and finally blocks) yy
30
LoggingCR ehlPErrors/exceptions are logged?n
31
Error, displaying contentsCR S ErrDDo not display internal information to clients.??
32
Exception typeCR ETWrong/not the right type of exception is used/thrownnn
33
Security
34
Logging, storing contentsCR S ErrLDo not log sensitive data. (Technical usernames, like db access user, or passwords etc.)nana
35
Information on User tokensCR UM existingdo 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.nana
36
Password storingCR UM pswddo not store password in clear text. Use latest techniques of password storing (hash and random salt).nana
37
Optimization
38
Optimization generalCR OptAvoid 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.nana
39
Comments
40
Comments (XML comment)CR C XMLAll 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.
nn
41
Comments in codeCR C CodeOnly in extreme and valid cases. One case is 3rd party (or not possible to refactor) component usage with not CC interfaces.nn
42
Comment, missingCR C MIndicate if a comment is missing.Dev at least should inspect the validity of the request.ny
43
Other
44
OtherCR XIndicates 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.
nn
Loading...
Main menu