Code Review

Phase 1: The Light Pass

General

• Code formatting has been used
• Line indentation char and line breaks
• Only required files are changed
• Disagreement between code and specification
• Comments in source code are included
• Documentation of the implementation is created or updated
• Does not contain any unimplemented areas like //TODO or //FIXME
• Optimistic or undefensive programming

Unclear Or Messy

• Verify correct and meaningful naming
• Magic numbers and values
• Variables used for more than one purpose
• Minimized variable, method and class scopes
• Method signature
• Packing too much into one line
• Complex IF conditions, IF instead of Switch
• Cyclomatic complexity

Error Handling

• Use exceptions only for unexpected errors
• Avoid empty catch blogs
• Error handler over-catches exceptions and aborts current flow or application
• Error handler is not implemented e.g. contains TODO, FIXME

Java

• Review class imports
• Wrong use of == and equals()
• Wrong use of collection data types, like List instead of Set
• Object contract errors (e.g. equals and hashCode)
• Exposure to immutable data types

Git Commit Message

• Describes what and why it has changed
• Verify correct format

Phase 2: The Contextual Pass

Functional programming

• Prefer immutability
• Minimize side-effects and perform them in a central place
• Do not rely on global state
• Plan for composing functions
• Keep method signatures as simple as possible
• Write generic functions
• Avoid type specific functions

Code Structure

• Understandability of written changes
• No logical errors
• Max usage of static type compiler checking
• Does not violate architecture guidelines, design principles or implementation patterns
• Are there any alternative implementations that increase simplicity, readability or maintainability
• Check edge cases in functions
• Any better approach to use a framework, library or class exists?
• Look for omissions: Shouldn't this component also do X?
• Enough log statements to find bugs quickly

External Systems

• Reduce amount of calls / Optimize calls to external systems

Tests

• Unit-tests and End 2 End Tests are added and test all functionality
• Test coverage of changed lines and critical path

Code-Review Commenting

• Adding comments: Be polite and constructive
• Add positive comments for good code e.g. unusually elegant solution, creative solution, great design

Finalizing

• Sign of the pull request

References

Code Review

Code Briefing: What does it mean when code is “easy to reason about”?

Do Not Allow Bad Smells In Your Code

Writing Great Git Commit Messages; A Revision

Giving better code reviews

Coding like Shakespeare: practical function naming conventions

Simple Testing Can Prevent Most Critical Failures: An Analysis of Production Failures in Distributed Data-Intensive Systems

Software Architecture and Design

YouTube: Core Design Principles for Software Developers

Information about properly formatted commit messages

Replacing Throwing Exceptions with Notification in Validations

The Dao of Immutability