This is a simple check-list to make code reviews more valuable. Simply check these rules.

Download a single page word document: Code Review Cheat Sheet

What % of code reviews should pass the first time?

About 10% should pass the first time. The other 90% of the time, a code review should fail. Now this low 10% percentage of first-time successful code reviews might increase as a team matures and members follows these rules as a behavior, but if you start to see really high first-time successful reviews, you might need to re-focus on this article.

However, if 100% or even 80% of code reviews are passing without comments, then you are likely not code reviewing correctly.

Does the code follow the 10/100 Rule?

This is a quick check rule that isn’t extremely rigid. See the 10/100 rule of code

This rule is what I call the S.O.L.I.D. training wheels. For more on S.O.L.I.D., see below. Code often follows all other rules naturally if it follows the 10/100 Principle.

Method has less than 10 lines

Is the method that was added or changed 10 lines or less? (There are always exceptions such as Algorithms)

100

Is the class 100 lines or less?
Note: Model classes should have zero functions and be closer to 20 lines.

Existing code that breaks the 10/100 Rule

Did the class get smaller? We don’t make it bigger.

Is the code S.O.L.I.D.

S.O.L.I.D. is an acronym. See this link: https://en.wikipedia.org/wiki/SOLID

Single Responsibility Principal

Does each class have a single responsibility? Does each method have a single responsibility?
Is this the only class that has this responsibility? (No duplicate code or D.R.Y. (Don’t Repeat Yourself)

Open/Closed Principle

Can you extend the functionality without modifying this code? Config, Plugins, event registration, etc.
Is there configuration in this code? If so, extract it. Configuration does not belong in code.
Encapsulation. Is everything private or internal except the few things that should be published as public?

Liskov substitution principle

Is inheritance used? If so, does the child type cause issues the parent type wouldn’t cause?

Interface segregation principle

Does the code use interface-based design?
Are the interfaces small?
Are all parts of the interface implemented without throwing a NotImplementedException?

Dependency inversion principle

Does the code reference only interfaces and abstractions?
Note: If new code references concrete classes with complex methods, it is coded wrong.

Cyclomatic Complexity

Cyclomatic Complexity is the number of linearly-independent paths through a program module. Basically, if statements, conditions, etc. Higher is bad.

Are there too many conditions or branches in a piece of code? If so, that can make it buggy. Usually you never have high Cyclomatic Complexity if you follow the 10/100 Principle.

Is the code Unit Tested

99% coverage

Is the Code 99% covered? Is code not covered marked with the ExcludeFromCodeCoverageAttribute?

Test Names

Are tests using proper names: <ClassName>_<MethodName>_<State>_<Result>

AAA Pattern

Tests should be written with Arrange, Act and Assert parts clearly marked, often with comments.

Parameter Value Tests for methods with parameters

Are all parameter values that could cause different behavior covered?
See these links:
Unit testing with Parameter Value Coverage (PVC)
Parameter Value Coverage by type

Test Example

Example in C#: Imagine a class called MyClass with a method: string DoSomething(string s);

public void MyClass_DoSomething_ParameterSNull_Throws()
{
   // Arrange
   var myClass = new MyClass();
   var expected = "SomeExpectedValue";
   // Act
   var actual = myClass.DoSomething();
   // Assert
   Assert.AreEqual(expected, actual);
}

Naming things

Typos

Are your names typo free?

Naming convention

Do your file names, class names, method names, variable names match existing naming conventions?

Specific Names

Are the names specific. Avoid general words “SomethingManager” or “AbcHelper”. General names should be avoided. Specific names are self-documenting.

Big O

Do you have any glaringly obvious Big O problems? n or n² vs when it could be constant or log n.
See: https://en.wikipedia.org/wiki/Big_O_notation

String Handling

Are you handling strings safely?

Are there magic strings? Do you understand the difference between volatile (untrusted strings) and nonvolatile (trusted strings).

Documentation

Did you write documentation? Is every public method/property/field documented?

Self-review

Has the submitter reviewed their own code. I understand that some tools don’t allow you to submit a review for your own change. That is not what I am talking about. I am talking about you have reviewed your own code, added comments anticipated questions from reviewers, and made sure the code meets the code review check-list yourself.

You should be failing your own reviews once or twice before anyone else even has a chance to look at them. If the tool doesn’t support a self-review, then leave a comment, saying: I have reviewed this myself and it is ready for others to review.