Code Review SOME USEFUL GUIDELINES The Attitude We

Code Review SOME USEFUL GUIDELINES

The Attitude We want to improve team communication and give feedback We want to improve code & get to a place of writing better code Also focus on good things We don’t want to focus on one-off problems We don’t want to focus on the person we’re reviewing We don’t want to redesign code

Look out for: Magic Numbers and Strings If Statement structure Create them as fields at the top of the class Use brackets {} to be safe String concatenation “We” + “NEVER” + “want” + “this” Use String. Builder or String. format Use readonly, not const They may seem similar but they’re not

Look out for Bad IDisposable usage Enum Initialization Always have a value “Unknown” or “Invalid” = 0 Large method bodies Wrap in using statements You can generally find a way to chop up methods into more atomic parts Interface vs Abstract Classes Abstract = “is a” Interface = “can do”

Look out for Excess Derivation Often member data is good enough to indicate what type a class is Layers add complexity we often don’t need Avoid nesting classes Avoid singletons model Use Static constructors instead Don’t use new, use override Use Auto properties when you can

Asp. Net concerns Use Encoding Markup in files <meta charset=“utf-8”> saves lives Edge browser versions If you’re using latest technologies, specify you want latest browser engine <meta http-equiv=“X-UA-Compatibile” content=“IE=edge; chrome=1”> Don’t embed JS or CSS in a page Use CDN (Content Delivery Networks) instead of locally serving common libraries

Asp. Net concerns Concatenate CSS using bundling Caching done per request Often case we have a limit of 10 CSS files Lifetime is never a certain thing inside the Web. Api Overuse of Web API Attributed Routing The exception, not the rule

Entity Framework Lazy Loading Entity Framework result-set sizes Make sure to resolve IQueryable before returning to client, and Include any relevant info using. Include() Put limits on queries and where possible add paging from the get-go Big EDMX problems Have a bunch of smaller separate EDMX’s for different parts of the system.

Architecture (yes this can be reviewed too) Not Invented here syndrome Too Many Layers for Abstraction for its own sake is seldom good Integration problems Use what already exists Unit testing! Dependency Injection solves a lot of problems Can also create circular reference problems