Code Review Template - DOC

Document Sample
Code Review Template - DOC Powered By Docstoc
					Code Review Minutes
Date of Review              MM / DD / YYYY

Item(s) to Review

Description of Item(s),
Background and
Context

Objectives of Review

Owner(s) of Item(s)

Who is Impacted? Who
Depends on these
Items?

Reviewed by

Next Steps




Prior to the Review
 Review Established Coding Standards
 It’s generally a good idea to adhere to the Microsoft Coding Standards. Therefore, take some time before your code
 review to skim through the content provided at the following links.

 Design Guidelines for Class Library Developers
 http://msdn2.microsoft.com/en-us/library/czefa0ke(vs.71).aspx

 Microsoft’s Internal Coding Standards
 http://blogs.msdn.com/brada/articles/361363.aspx

 Coding standards should be considered to be guidelines rather than strict rules. That being said, developers should try
 to follow these recommendations as closely as possible … your lives will be much easier as a result.

 Use FxCop
 FxCop is a tool that will help you to improve the quality of your code. To read more about it visit the following link …

 http://msdn.microsoft.com/library/default.asp?url=/msdnmag/issues/04/06/Bugslayer/toc.asp


 Get a Copy of Steve McConnell’s “Code Complete”
 Steve McConnell has written a number of “classics”, and his book “Code Complete” will be sure to help you not only at
 Monster, but throughout your career …

 http://www.amazon.com/gp/product/0735619670/103-3384042-5349424?v=glance&n=283155

 Skim Through Microsoft’s Recommendations on Improving .Net Application Performance
 The list of items that might be reviewed to assess performance issues is quite large an ever changing. For a detailed
 exploration of .Net Application Performance visit …

 http://msdn2.microsoft.com/en-us/library/ms998530.aspx
DesignPatternsFor.Net Code Review Template                 Printed 6/10/2010 10:19:00 PM                        Page 1 of 7
Code Review Minutes
Date of Review                 MM / DD / YYYY


Logical Design and Layering Approach
Provide commentary on the solution design highlighting appropriateness for problem domain, advantages, and potential
trade-offs.




Maintainability, Adaptability
Assess factors that will impact the ease with which the software may be maintained or altered over time.

Area of Review                      Observations                               Suggestions
Duplicated Code                                                               Whenever you see code that looks like it
                                                                              was copied and pasted, even with a few
                                                                              minor changes, explore opportunities for
                                                                              reuse. You might consider implementing
                                                                              classes that encapsulate the logic, or
                                                                              creating helper classes.

Use of Short Methods                                                          Try to keep all methods short. Long
                                                                              methods become very hard to maintain
                                                                              over time

Variable Scoping                                                              Try to keep variables scoped to the lowest
                                                                              level possible. Global variables should be
                                                                              avoided.

Cohesion of Logic in Classes                                                  Do everything you can to encourage high
                                                                              cohesion in all classes.

Coupling: Long Parameter Lists
Generally, methods with long
parameter lists create higher
degrees of coupling and
therefore decrease
maintainability

Coupling: Control Coupling                                                    If you use “control flags” to drive the
                                                                              internal behavior of a method, explore
                                                                              opportunities for specialized classes or
                                                                              overloaded methods

Coupling: Global Data Coupling
This typically occurs when
global variables are used to
drive the behavior of a group
of classes

Coupling: Solution Sprawl
Across Classes
DesignPatternsFor.Net Code Review Template                Printed 6/10/2010 10:19:00 PM                       Page 2 of 7
Code Review Minutes
Date of Review               MM / DD / YYYY

When you need to make
changes across a large number
of classes in order to
implement a change in the
application’s behavior, you’ve
got solution sprawl.

Coupling: Inter-Layer
Dependencies
The more method calls you
have from one class in a given
assembly or “logical layer” to
other classes in a different
assembly or “logical layer”, the
tighter the coupling, and the
harder it will be to maintain
over time.

Conditional Complexity, Level                                    Consider applying the “Extract Method”
of Nesting, Use of Flags, use of                                 refactoring to move code from within an
switch statements                                                “If Block” to a method that describes what
If a method has deeply nested                                    that code does.
“If” statements, uses flags
(e.g. Booleans, etc.) to drive                                   Whenever you see switch statements or
logic, it can become very                                        “If” statements in a class, you might have
difficult to read and maintain                                   an opportunity to use class specializations
                                                                 instead.

Encapsulation, Information
Hiding, Inappropriate Intimacy
between Classes
When classes know too much
about the internals of each
other, they become very tightly
coupled and hard to maintain.


Magic Numbers and Literals                                       Try to replace magic numbers and literals
Magic Numbers and literals are                                   with constants that have meaningful
numeric or alpha-numeric                                         names.
values in the code whose
meaning may not be self-
explanatory.


Speculative Generality
A.K.A. You Aren’t Going to
Need It

Versioning Approach
Has the developer produced an
approach that may be easily
versioned over time?

DesignPatternsFor.Net Code Review Template    Printed 6/10/2010 10:19:00 PM                      Page 3 of 7
Code Review Minutes
Date of Review               MM / DD / YYYY

Use of Interfaces
Are interfaces used
appropriately?

Simplicity of Solution


General readability and
intuitive naming of fields,
properties, variables, methods,
etc.

Appropriate Use of Comments


General adherence to Microsoft
Coding Standards

Unit-Tests were created to
support regression testing




Robustness
Assess the primary factors that affect how well the software handles incorrect data or unforeseen scenarios.

Area of Review                     Observations                                 Suggestions
Defensive Programming
Performs early parameter
checking (e.g. boundary
checks, type checks,
assertions, etc.) before
executing main body of logic

Checks return values received
from service or method calls

Checks for nulls when
appropriate

Avoids “Apocalypse Ready”
designs; These are designs
that handle exceptions that will
probably never happen or
“fringe case” issues
Proper use of Exception
Handling
All exceptions are caught and
handled at the “top of call-
stack”

Lower in the call stack,
exceptions are only caught to
DesignPatternsFor.Net Code Review Template                Printed 6/10/2010 10:19:00 PM                        Page 4 of 7
Code Review Minutes
Date of Review               MM / DD / YYYY

log or gather information, add
information to the exception,
perform cleanup, or attempt to
recover

Prefer the use of standard
framework-defined exceptions
when possible

Exceptions are thrown only for
clearly abnormal cases;
Exceptions aren’t used to
control application flow.

Exceptions are Logged to
Facilitate Debugging

Parameters are strongly typed

Assess Potential for Data Loss
Due to “Shortening
Conversions”
This occurs when you attempt
to cast larger types like longs
to smaller types like integers.



Performance
Assess the areas that will typically have the greatest impact on application performance.


Area of Review                     Observations                                  Suggestions
Style of Communication
Code has minimal cross-
machine calls

Code has minimal cross-
process calls

Code favors chunky vs. chatty
communications to services

Avoidance of “Data Buffet”
anti-pattern; This occurs when
a query or object retrieves
more data than it’s consumer
will probably use

Assess ADO.Net Related Code
for Performance Issues



DesignPatternsFor.Net Code Review Template                 Printed 6/10/2010 10:19:00 PM       Page 5 of 7
Code Review Minutes
Date of Review                 MM / DD / YYYY


Evaluate use of Boxing /                                           Avoid conversion to/from value and
Unboxing                                                           reference types where possible

Loop considerations
Loops are exited as soon as
conditions met

Expressions are not re-
evaluated from within the loop
controller statements

Logic that always gets same
results does not occur within
the body of loops

Use “For” instead of “For Each”
when appropriate

String Handling
Assess string concatenation
approaches

Assess use of StringBuilder

Resource Cleanup
Resources are acquired late
and released early

Assess potential for Generation
2 garbage collection

Evaluate appropriate use of
“using” statement

The Dispose pattern is
implemented for managed
resources

Finalizers are avoided

Appropriate Use of Caching
Items that change frequently
are not cached

Know your cache-hit ratio;
don’t bother caching items that
aren’t retrieved frequently

  Appropriate use of ViewState
  and Postback checking

  Consider opportunities for
  Asynchronous or Queued
DesignPatternsFor.Net Code Review Template      Printed 6/10/2010 10:19:00 PM                    Page 6 of 7
Code Review Minutes
Date of Review              MM / DD / YYYY

  Operations

  Solution makes minimal or
  no use of Reflection

  Use of Code Instrumentation

  Premature Optimization?



Supporting Documentation
Assess the design solution’s need for supporting documentation

Area of Review                    Observations                                 Suggestions
  Object Models

  Sequence Diagrams

  Entity-Relationship Diagrams

  Other Diagram

  Use of nDoc or XML
  Comments

  Where will Documentation be
  Stored? How will it be
  Maintained?


Migration Considerations
Assess how current or future users (i.e. developers) dependent on this solution will migrate to the proposed approach.
How will the risk of breaking changes be minimized?




Other Areas to Consider
Provide commentary on any other aspect of the design solution.




DesignPatternsFor.Net Code Review Template                Printed 6/10/2010 10:19:00 PM                       Page 7 of 7