Code review — what to look for — Hinglish notes

4.5.18 · Coding › Software Engineering

WHY code review bilkul exist karta hai?

WHY yeh kaam karta hai (first principles):

Author apni khud ki assumptions ke liye blind hota hai — woh woh code "dekhta" hai jo usne likhna chaha tha, na ki woh code jo usne actually likha. Fresh pair of eyes ke paas aisa koi blind spot nahi hota.
Bugs exponentially zyada expensive hote jaate hain jitna der se unhe pakda jaaye. Rough cost ladder:

$cost_{fix} \approx c_{0} \cdot k^{s}$

jahan $s$ stage hai (0 = review par, 1 = QA mein, 2 = production mein) aur $k > 1$ (aksar $k \approx 10$ quote kiya jaata hai). Toh review par bug pakadne par ( $s = 0$ ) cost $c_{0}$ hai, lekin production mein ( $s = 2$ ) yeh $\approx c_{0} k^{2} = 100 c_{0}$ ho jaati hai. WHY review kaafi hai: tum bugs ko exponent mein neeche le jaate ho.

WHAT dekhna hai — priority ladder

Review top-down karo: pehle high-impact, hard-to-reverse cheezein. Agar tum correctness check karne se pehle spacing nit-pick karte ho, to tumne galat cheez optimize ki (80/20!).

Correctness — kya yeh woh karta hai jo yeh claim karta hai? Edge cases: empty input, null, off-by-one, max/min values, concurrency.
Design / architecture — kya yeh system mein fit hota hai? Sahi jagah, sahi abstraction, koi existing utility reinvent nahi ho rahi. Baad mein fix karna sabse mushkil → highest priority.
Security — input validation, code mein koi secrets nahi, SQL injection, authz checks.
Tests — kya naye tests actually naye behaviour ko exercise karte hain (failure path bhi include)?
Readability / naming — kya koi newcomer 6 mahine baad yeh samajh sakta hai?
Performance — sirf wahan jahaan matter karta hai (hot loops, big- $O$ regressions), premature nahi.
Style — yeh ek linter/formatter ko karne do; insaanon ko nahi karna chahiye.

HOW actually karna hai (practice)

HOW, step by step:

Pehle PR description + linked ticket padho — problem jaane bina solution judge nahi kar sakte.
Forecast karo ki diff mein kya hoga. Phir padho aur verify karo (Forecast-then-Verify).
Change locally pull karo / pehle tests padho — woh intended behaviour bataate hain.
Happy path trace karo, phir deliberately edge cases hunt karo.
Blocking comments aur preferences mein fark karo. Nits clearly prefix karo: nit:.

Worked examples

Worked example Example 1 — off-by-one pakadna

def last_n(items, n):
    return items[len(items) - n:]   # return last n items

Kyun review karein? Happy path par theek lagta hai. Forecast: agar n > len(items) ho? Tab len(items) - n negative hoga, toh slicing end se wrap ho jaayegi — silently galat items return karega. Yeh step kyun? Edge cases (n list se bada) wahan hain jahan correctness bugs chupta hai. Fix: return items[-n:] if n > 0 else [] aur n > len(items) ke liye test add karo.

Worked example Example 3 — security

query = f"SELECT * FROM users WHERE name = '{name}'"

Kyun flag karein? name user input se aata hai → classic SQL injection. Yeh step kyun? Security style se zyada important hai; ek injected '; DROP TABLE users; -- catastrophic hai. Fix: parameterized query use karo: cursor.execute("... WHERE name = %s", (name,)).

Common mistakes (Steel-man + fix)

Recall Feynman: ek 12-saal ke bachche ko explain karo (reveal karne ke liye click karo)

Socho tumne school mein ek essay likha aur submit karne se pehle tumhara dost padha. Tum apni khud ki spelling mistakes nahi dekh sakte kyunki tumhara brain woh padhta hai jo tumne socha tha. Tumhare dost ki fresh eyes hain aur woh unhe pakad leta hai. Code review exactly wahi hai — lekin unnn instructions ke liye jo hum computer ko dete hain. Tumhara dost pehle sabse badi cheezein check karta hai ("kya tumhari story ka koi sense bhi hai?") chhoti cheezein se pehle ("tumse ek comma miss hua"), kyunki ek confusing story ek missing comma se bahut buri hai.

Active-recall flashcards

Code review kya hai?

Code change ki systematic examination kisi aur dwara (author nahi), merge hone se pehle.

Author apne khud ke bugs kyun reliably nahi pakad sakta?

Woh woh code dekhta hai jo usne likhna chaha tha, na ki jo actually likha — woh apni khud ki assumptions ke liye blind hota hai.

Kis order mein review karna chahiye?

Correctness → Design → Security → Tests → Readability → Performance → Style (CD STeReo PerSon).

Correctness/design ko style se pehle kyun review kiya jaata hai?

Yeh baad mein fix karna sabse expensive hota hai aur reverse karna sabse mushkil; style ko linter se automate kiya ja sakta hai.

Bug-fix cost stage ke saath kyun badhti hai?

Yeh roughly

c_{0} k^{s}

ke roop mein — exponentially — badhti hai, toh ek production bug (

s = 2

) review-stage bug ka ~

k^{2}

guna cost kar sakta hai.

Reviewer time

R

kab worth it hai review?

Jab

R < n p c_{0} (k^{2} - 1)

— zyada bugs, better reviewer, costlier failures, sab reviewing ke favour mein hain.

Pehle tests kyun padhne chahiye?

Woh intended behaviour document karte hain, yeh bataate hain ki is change ke liye "correct" ka matlab kya hai.

Steel-man "tests pass toh correct hai" — fix?

Tests sirf woh cases cover karte hain jo kisi ne likhe; poochho kaunse inputs COVERED nahi hain aur woh test maango.

Large PRs split kyun karni chahiye?

~200–400 lines ke baad reviewer defect-detection collapse ho jaati hai; small diffs ko real scrutiny milti hai.

Tum kiske baare mein comment karte ho, kiska rule hai?

Code/change par comment karo, kabhi person par nahi — author ko motivated rakho, defensive nahi.

Connections

Unit testing — tests woh hain jo tum verify karte ho; review tests khud ko check karta hai.
Pull requests and Git workflow — woh mechanism jis par reviews chalte hain.
Technical debt — review mein design comments debt-prevention hain.
SQL injection and input validation — review ki security layer.
Big-O notation — performance regressions spot karne ke liye zaroori.
Refactoring — readability comments aksar small refactors request karte hain.