3.3 Konstruktív kommentek

← 3. Code review kultúra és készségek

3.3 Hogyan írj konstruktív review kommentet

Áttekintő

A code review nem csak arról szól, hogy megtaláld a hibákat – arról is szól, hogyan kommunikálod azokat. Egy rosszul megfogalmazott komment demotiválja a fejlesztőt, védekezővé teszi, és rontja a csapaton belüli bizalmat. Egy jól megírt komment viszont tanít, segít, és hosszú távon jobb fejlesztőt csinál a kollégádból.

Ebben az alfejezetben megtanulod:

  • milyen típusú kommentek léteznek és mikor melyiket használd
  • hogyan fogalmazz úgy, hogy ne sérts meg senkit
  • mi a nit: előtag és miért szeretik a profi csapatok
  • hogyan adj pozitív visszajelzést (és miért fontos ez)
  • konkrét példákon keresztül: rossz komment → jó komment átírások

Miért számít ez az állásinterjún?
A legtöbb tech interjún megkérdezik: „Hogyan adsz visszajelzést a kollégáidnak?" vagy „Mesélj egy code review szituációról." Ha tudod elmagyarázni a különböző kommenttípusokat és a mögöttük lévő gondolkodást, az egyből megkülönböztet a többi junior jelölttől.

Részletes leírás

A három kommenttípus

A profi fejlesztői csapatokon belül az elfogadott norm az, hogy a review kommentek jelzik a súlyukat. Nem minden észrevétel egyformán fontos – és ezt a szerzőnek is értenie kell, hogy jól priorizáljon.

1. Blocker (kötelező javítás)

Ez az a komment, ami miatt nem lehet mergelni a PR-t. Valódi probléma: biztonsági rés, logikai hiba, production-t törő bug.

Jellemzők:

  • Egyértelmű, miért blokkoló
  • Konkrét javaslatot tartalmaz a javításra
  • Nem személyes, a kódról szól

2. Javaslat (opcionális, de ajánlott)

Nem blokkolja a merge-t, de érdemes megfontolni. Lehet jobb megközelítés, olvashatóbb megoldás, jobb névválasztás.

Jellemzők:

  • Feltételes hangnem: „Érdemes lehet...", „Mi lenne ha..."
  • Megmagyarázza, miért jobb az alternatíva
  • Nem kötelez semmire

3. nit: (nit-pick, apróság)

A nit: (nitpick rövidítése) egy elterjedt konvenció a tech iparban. Olyan apróságokat jelöl vele a reviewer, amik teljesen elhagyhatók – de ha már így is bent van a kezed a kódban, mégis jobb lenne így.

nit: ezt a változót `isLoading`-ra nevezhetnéd `loadingState` helyett

Miért jó ez a konvenció?

  • A szerző tudja, hogy nem kell foglalkoznia vele, ha más a prioritása
  • A reviewer mégis megoszthatja a véleményét anélkül, hogy felesleges nyomást gyakorolna
  • Átlátható a kommunikáció

Hogyan fogalmazz, hogy ne sérts meg senkit?

A code review egyik legnagyobb csapdája: a kód kritikája személyes kritikának tűnik.

Ez különösen igaz juniorokra, akik még nem választják szét magukat és a kódjukat. Ha azt mondod valakinek, hogy „ez rossz", könnyen úgy érzik: „én vagyok rossz".

Az alapszabály: mindig a kódról beszélj, nem a személyről.

❌ Személyes✅ Kódközpontú
„Ezt rosszul csináltad"„Ez a megközelítés problémát okozhat, mert..."
„Miért írtad így?"„Ezt lehetne így is megoldani: [példa]"
„Ez nyilvánvalóan hibás"„Úgy látom, itt az X eset nincs kezelve"
„Ezt mindenki tudja"„Ezt érdemes tudni: [magyarázat]"

Tippek a megfogalmazáshoz:

  1. Használj „mi" formát – „Ha ezt így csináljuk, akkor..." – közös ügy, nem ítélet
  2. Kérdezz, ne állíts – „Mit szólsz, ha inkább X-et használunk?" vs. „X-et kell használni"
  3. Magyarázd a miértet – Nem elég azt mondani, hogy „ez rossz". Miért rossz? Mi a következménye?
  4. Adj alternatívát – Ha kritizálsz, adj egyben javaslatot is
  5. Ismerj el kontextust – „Látom, hogy ez a jelenlegi architektúrába nehézkesen illik..." – empátia

Pozitív visszajelzés: a legtöbbször kihagyott eszköz

A legtöbb fejlesztő kizárólag hibákat ír a review-ba. Ez hatalmas hiba.

Miért fontos a pozitív visszajelzés?

  • Erősíti a jó szokásokat (amit dicsérünk, azt megismétlik)
  • Csökkenti a review-tól való félelmet
  • Embert csinál belőled, nem csak hibakereső gépet
  • Jobb csapatkultúrát épít

Ez nem azt jelenti, hogy minden sorhoz dicséretet kell írni. De ha látsz valami tényleg elegáns megoldást, egy jó névválasztást, egy ügyes tesztet – mondd ki.

Jó megközelítés! Ez sokkal olvashatóbb, mint amit korábban csináltunk.
Tetszik, hogy ezt a sarokesetet is lekezeltük – általában ez szokott elsikkadni.

Ez 10 másodperc és rengeteg goodwill.

A komment struktúrája

Egy jól megírt review komment általában három részből áll:

  1. Mit látsz (tény, nem ítélet)
  2. Mi a probléma / miért számít (kontextus)
  3. Mit javasolsz (alternatíva vagy irány)

Nem kell mindhárom minden esetben – de ha blocker kommentet írsz, ez a minimum.

Életszerű példák

Példa 1: Biztonsági probléma

Rossz komment:

Ez SQL injection! Erre nem gondoltál?

Jó komment:

[Blocker] Ezen a ponton az SQL query összefűzéssel épül fel a user inputból,
ami SQL injection sérülékenységet jelent. Érdemes paraméterezett lekérdezésre
(prepared statement) cserélni – a részlegünkön így szokás: [link vagy példa].

Példa 2: Olvashatóság

Rossz komment:

Ez a függvény értelmezhetetlen.

Jó komment:

nit: Ez a függvény elég komplex – egy rövid kommenttel vagy a változónevek
pontosításával sokat javíthatnánk az olvashatóságon. Mi lenne ha a `d` változót
`daysSinceLastLogin`-ra neveznénk?

Példa 3: Felesleges duplikáció

Rossz komment:

DRY elv! Nem hallottál róla?

Jó komment:

Ez a logika nagyon hasonlít arra, amit a `UserService`-ben is megcsináltunk
(lásd: `calculateDiscount()` metódus). Érdemes lehet kiemelni egy közös helyre,
hogy ha változik, elég egy helyen módosítani.

Példa 4: Hibakezelés hiánya

Rossz komment:

Mi van ha ez null? Gondoltál erre?

Jó komment:

[Javaslat] Ha a `fetchUser()` null-t ad vissza (pl. nem létező user ID esetén),
a következő sor NPE-t dob. Érdemes lehet egy `if (user == null) return;` check-et
betenni, vagy a hívó oldalon kezelni ezt az esetet.

Példa 5: Pozitív visszajelzés

Szép megoldás! Korábban ezen a részen mindig sokat gondolkodtunk –
ez az approach sokkal egyszerűbb és könnyen követhető.

Példa 6: Performancia aggály

Rossz komment:

Ez lassú lesz.

Jó komment:

[Javaslat] Ez a ciklus minden iterációban meghívja az adatbázist – ha a lista
nagy (pl. 1000+ elem), ez komoly performance problémát okozhat (N+1 query).
Mi lenne, ha egy lekérdezéssel előtöltenénk az összes szükséges adatot?

Feladatok és tesztek

Gyakorlati feladat: 5 rossz komment átírása

Az alábbi 5 kommentet írd át konstruktív, professzionális formába. Ahol szükséges, adj blocker/javaslat/nit jelzést is.

1. feladat:

Eredeti komment:

Ez teljesen felesleges komplexitás. Miért ilyen bonyolultan csináltad?

Kontextus: A junior egy 40 soros helper osztályt írt, amit 3 sorban is meg lehetett volna oldani.

2. feladat:

Eredeti komment:

A tesztek hiányoznak!!!

Kontextus: A PR-hoz nem tartoznak unit tesztek, de a csapat policy szerint kellene.

3. feladat:

Eredeti komment:

Ezt soha nem szabad így csinálni. Ez alapvető hiba.

Kontextus: A fejlesztő hardcode-olt egy API kulcsot a kódba.

4. feladat:

Eredeti komment:

Jó, rendben.

Kontextus: A fejlesztő egy ügyesen megírt cache megoldást rakott be, amit a reviewer aprólékosan átnézett.

5. feladat:

Eredeti komment:

Ez a változónév értelmetlen. Ki érti ezt el?

Kontextus: A változó neve tmp2 egy 80 soros függvényen belül.

Tesztkérdések

1. kérdés: Melyik komment tekinthető hatékonynak és konstruktívnak?

a) „Ez hibás implementáció."
b) „Ez a megközelítés problémát okozhat edge case-eknél – ha X feltétel teljesül, Y történik. Mi lenne ha Z-t próbálnánk?"
c) „Miért ilyen bonyolultan oldottad meg ezt?"
d) „Mindenki tudja, hogy ezt így kell csinálni."

2. kérdés: Mit jelent a nit: előtag egy review kommentben?

a) A komment blokkolója a merge-nek
b) A komment egy kötelező biztonsági javításra vonatkozik
c) A komment apróság, nem kötelező foglalkozni vele, de felhívja rá a figyelmet
d) A reviewer nem ért egyet a megközelítéssel, de elfogadja

3. kérdés: Miért fontos pozitív visszajelzést is adni review során?

a) Mert kötelező a céges policy szerint
b) Mert erősíti a jó szokásokat és jobb csapatkultúrát épít
c) Mert különben a reviewer rossznak tűnik
d) Csak akkor szükséges, ha a junior fejlesztő érzékeny

4. kérdés: Melyik a megfelelő sorrend egy jól megírt blocker kommentben?

a) Javaslat → probléma → tény
b) Tény → probléma/következmény → javaslat
c) Személyes vélemény → kritika → elvárt megoldás
d) Link → magyarázat → kérdés

5. kérdés: Az alábbi kommentek közül melyik a „javaslat" típusú (nem blocker)?

a) „[Blocker] Ez SQL injection sérülékenységet jelent, mielőtt ez mergel, javítani kell."
b) „nit: Ezt a változót talán jobb lenne isVisible-re nevezni."
c) „Érdemes lehet megfontolni egy Factory pattern használatát itt – nem kötelező, de karbantarthatóbbá tenné."
d) „Szép megoldás, sokat egyszerűsített a korábbi kódhoz képest."

6. kérdés: Mi a legnagyobb probléma ezzel a kommenttel: „Ezt mindenki tudja, hogy nem szabad így csinálni."?

a) Túl rövid
b) Személyes hangvételű és nem ad konkrét segítséget
c) Hiányzik a nit: előtag
d) Nem tartalmaz linket a dokumentációra

7. kérdés: Melyik megfogalmazás ösztönzi inkább a konstruktív párbeszédet?

a) „Ez teljesen rossz megközelítés."
b) „Miért gondoltad, hogy ez működni fog?"
c) „Mit szólnál, ha X megközelítést próbálnánk? Az Y okból hatékonyabb lehet."
d) „Ez nem az elvárható minőség."

8. kérdés: Egy junior fejlesztő PR-jában minden rendben van, a kód jól megírt és tesztelt. Mit tegyen a reviewer?

a) Semmit, csak approve-olja komment nélkül
b) Írjon egy rövid pozitív megjegyzést, pl. „Jól megírt, köszönöm!" – és approve-olja
c) Keresse azokat a részeket, ahol még javítható lenne, hogy legyen érdemi tartalma a review-nak
d) Kérdezze meg a seniortól, hogy tényleg jó-e

9. kérdés: Melyik mondat fejezi ki legjobban a „kód ≠ személyiség" elvét?

a) „Látom, hogy ez a fajta megközelítés szokott problémát okozni ilyen szituációban."
b) „Te mindig így csinálod, és mindig hibás lesz."
c) „Nyilván nem gondoltad végig alaposan."
d) „Én sosem csinálnám így."

10. kérdés: Mi a nit: komment legfőbb értéke a csapat számára?

a) Lehetővé teszi, hogy a reviewer minden kis hibát felsoroljon anélkül, hogy blokkolná a merge-t
b) Kötelezi a szerzőt, hogy az összes apróságot javítsa
c) Helyettesíti a blocker kommentet alacsony prioritású esetekben
d) Csökkenti a review jóváhagyáshoz szükséges időt

Megoldások (tesztkérdések)

  1. b – Konkrét, magyaráz, és javaslatot ad
  2. c – Apróság, nem kötelező foglalkozni vele
  3. b – Erősíti a jó szokásokat és jobb csapatkultúrát épít
  4. b – Tény → probléma → javaslat
  5. c – Feltételes hangnem, explicit nem kötelező
  6. b – Személyes hangvételű és nem ad konkrét segítséget
  7. c – Kérdés formájú javaslat, magyarázattal
  8. b – Pozitív visszajelzés is értékes, approve-olva
  9. a – A megközelítésről szól, nem a személyről
  10. a – Apróságokat lehet jelezni a merge blokkolása nélkül
Scroll to Top