DEV Community

mahesh_attarde
mahesh_attarde

Posted on

Notes on Code Review

"Build thing right" is motivation for code reviews. Depending on context, each of following point can be extended to any review.
My base understanding about reviewing someone code comes (Programmer Competency Matrix)
[https://sijinjoseph.com/programmer-competency-matrix/], which serves

Here is checklist I noted down from 2 finest engineers.

  • Size of Code Review

    • Is this Single Objective change?
    • Multiple Objective Change
      • is it time consuming and simple enough to review >15 min?
        • Else break it down
        • check anti-pattern, clubbing multiple checking?
  • Requested Functionality Check

    • Unit tests
      • are complete and correct ?
      • Missing tests?
    • Algorithm
      • Time Complexity
      • Space Complexity
      • Contextual Appeal of Algorithm
    • Data Structure
      • New Data Structure
        • Data Layout ?
        • Interfaces are stable?
        • Is there exposed state?
      • Existing Data structure
        • Is Data Structure Abused in usage?
      • Domain of Application
    • Find Assumptions hidden in place-sight?
    • Are there unhanded cases for assumptions?
  • Coding Anti-Patterns

  • Code Style Consistency

    • Style in Sync with existing style?
    • tool clang-format
  • Language features

    • Are there any Language Features abused?
    • Can there be scope for using language features?
  • Memory Issue

    • Is there chance of memory corruption?
    • Are there any data races?
    • tool val-grind
  • Performance Issue

    • Deterioration due to integration of Algorithmic Functionality
    • Performance Test
  • Concurrency Issue

    • Multi-threaded
    • Distributed
  • Contextual Appeal for Integration

    • Environment variable vs command line options
  • Risk of Code Change

    • Change affects area that is only requested , Guarded by Option with Default
    • is there a disaster scenario ?
    • are there deployment issues?
  • Architectural Change

    • Is there library dependency ? is it explicitly mentioned or implicit?
    • Is Architectural change adding value over time/ maintainance?
  • Review of "Language" (C++ in this Context)

    • Check Data types used, CV qualifiers, storage specifiers
    • Overflow and underflow conditions?
    • New Class
    • Checklist of Default constructors, operators
    • check behaviors are mocked with coverage
    • and follow up on guideline based on use cases.

Although this is checklist learnt from experienced programmer and awesome engineer known to me,
It may be possible to think of general framework after deep thought, which any rookie can use.

HTH!

Top comments (0)