3.2 Mit keresünk review során – a teljes ellenőrzőlista

Áttekintő

Amikor code review-t végzel – akár saját, akár mások kódján –, könnyen elveszhetsz a részletekben, ha nincs egy rendszer, ami mentén haladni tudsz. A tapasztalt reviewerek nem véletlenszerűen keresnek hibákat: tudják, mit, milyen sorrendben és miért érdemes ellenőrizni.

Ez az alfejezet egy praktikus ellenőrzőlistát ad, amit bármelyik programozási nyelvnél és csapatnál alkalmazni tudsz. Nem kell mindent minden PR-nál végigmenni – a kulcs a prioritizálás: mi blokkolja a merge-t, mi jelzés értékű, és mi csak nit (apróság).

Részletes leírás

A review célja – mielőtt belevágnál

Mielőtt a listát végigjárod, emlékezz az előző alfejezetből: a code review nem hibavadászat. A cél az, hogy a kód:

biztonságosan kerüljön a production-be
másnak is érthető legyen
hosszú távon karbantartható maradjon

A lista ennek mentén épül fel: a legsúlyosabb problémáktól (biztonság, korrekts működés) haladunk a kevésbé kritikusak felé (stílus, naming).

Az ellenőrzőlista – kategóriánként

1. Korrektség és logika (legfontosabb)

Mit nézz: Működik-e a kód arra a célra, amire szánták?

Kérdések:

Teljesíti-e az acceptance criteriát?
Lefedi-e az edge case-eket? (Üres tömb, null érték, 0, negatív szám, max érték)
Van-e off-by-one hiba ciklusokban?
Helyes-e a feltétel logikája? (Pl. >= vs >)

# Rossz: edge case hiányzik
def get_first_item(items):
    return items[0]  # Mi történik, ha items üres?

# Jobb:
def get_first_item(items):
    if not items:
        return None
    return items[0]

Miért számít: Ha a logika hibás, minden más mindegy – a kód nem azt csinálja, amit kell.

2. Biztonság

Mit nézz: Nyit-e a kód biztonsági rést?

Kérdések:

Van-e input validáció? Megbízhatunk a bejövő adatban?
Van-e SQL injection lehetőség? (Raw query felhasználói inputtal)
Jelenik-e meg szenzitív adat logban vagy hibaüzenetben?
Megfelelő-e a jogosultságkezelés? (Ki férhet hozzá az endpoint-hoz?)
Kerül-e API kulcs, jelszó, token a kódba hardcode-olva?

// Rossz: SQL injection lehetőség
const query = `SELECT * FROM users WHERE email = '${userInput}'`;

// Jobb: paraméteres lekérdezés
const query = 'SELECT * FROM users WHERE email = ?';
db.execute(query, [userInput]);

// Rossz: szenzitív adat logban
error_log("Login failed for user: " . $email . " password: " . $password);

// Jobb:
error_log("Login failed for user: " . $email);

Miért számít: Egy biztonsági rés production-ban súlyos következménnyel járhat. Ez az egyetlen kategória, ahol a review-t le kell blokkolni.

3. Hibakezelés

Mit nézz: Mi történik, ha valami rosszul sül el?

Kérdések:

Kezeli-e a kód a várható hibákat? (Hálózati hiba, adatbázis-hiba, fájl nem létezik)
Elnyelődnek-e a hibák észrevétlenül? (Üres catch blokk)
Milyen hibaüzenetet kap a felhasználó? (Informatív, de nem leplezetlenül technikai)
Logolva van-e a hiba, hogy nyomozni lehessen utána?

# Rossz: hiba elnyelve
try:
    result = call_external_api()
except Exception:
    pass  # Mi történt? Soha nem fogjuk megtudni.

# Jobb:
try:
    result = call_external_api()
except requests.Timeout as e:
    logger.error("API timeout: %s", str(e))
    raise ServiceUnavailableError("External service is temporarily unavailable")

Miért számít: Elnyelett hibák a legnehezebben debuggolható production problémák forrásai.

4. Olvashatóság és komplexitás

Mit nézz: Megérti-e egy másik fejlesztő 6 hónap múlva, mi történik?

Kérdések:

Érthető-e a kód magyarázat nélkül?
Nincs-e túl sok egymásba ágyazott feltétel? (3+ szint: refaktorálni kell)
Egy függvény egy dolgot csinál-e?
Ésszerű-e a függvény hossza? (30-50 sor felett érdemes törni)
Van-e szükségtelen komment, ami csak azt írja le, amit a kód neve már elárul?

// Rossz: mélyen egymásba ágyazva
function processOrder(order) {
    if (order) {
        if (order.items) {
            if (order.items.length > 0) {
                if (order.user) {
                    // végre csináljuk a dolgot
                }
            }
        }
    }
}

// Jobb: early return technika
function processOrder(order) {
    if (!order || !order.items || order.items.length === 0 || !order.user) {
        return;
    }
    // végre csináljuk a dolgot
}

Miért számít: A kódot egyszer írják, de tízszer olvassák. Az olvashatatlan kód technikai adósság.

5. Naming konvenciók

Mit nézz: Elmondja-e a név, mi a célja?

Kérdések:

Egyértelmű-e a változó neve? (data vs userOrderList)
Igével kezdődik-e a függvénynév? (get, calculate, validate, process)
Boolean változó neve igen/nem kérdés-e? (isActive, hasPermission, canEdit)
Konzisztens-e a névadás a többi fájllal? (camelCase vs snake_case – a projekt konvencióját kövesse)
Van-e rövidítés, amit csak a szerző ért? (usrMgr vs userManager)

# Rossz naming
def calc(x, y, f):
    tmp = x * y
    if f:
        tmp = tmp * 0.8
    return tmp

# Jobb naming
def calculate_price(quantity, unit_price, is_discounted):
    total = quantity * unit_price
    if is_discounted:
        total = total * 0.8
    return total

Miért számít: A rossz nevek arra kényszerítik a következő fejlesztőt, hogy a teljes kontextust újra felderítse.

6. Tesztek megléte és minősége

Mit nézz: Le van-e fedve a változás tesztekkel?

Kérdések:

Van-e teszt az új funkcióhoz?
Tesztelve van-e a happy path? Az edge case-ek?
A tesztek valóban elbuknak, ha a kód hibás? (Nem csak formálisan teljesülnek)
Nem tesztelnek-e implementációs részleteket ahelyett, hogy a viselkedést tesztelnék?

# Gyenge teszt: csak a happy path
def test_calculate_price():
    assert calculate_price(2, 10, False) == 20

# Teljesebb teszt:
def test_calculate_price():
    assert calculate_price(2, 10, False) == 20        # alap eset
    assert calculate_price(2, 10, True) == 16.0       # kedvezmény
    assert calculate_price(0, 10, False) == 0          # nulla mennyiség
    assert calculate_price(2, 0, False) == 0           # nulla egységár

Miért számít: Teszt nélkül nem lehet biztonságosan refaktorálni. A jövőbeli fejlesztő vak lesz.

7. Duplikált kód (DRY elv)

Mit nézz: Szerepel-e ugyanaz a logika több helyen?

Kérdések:

Van-e máshol a kódbázisban hasonló megvalósítás?
Ki lehet-e emelni egy függvénybe vagy helperbe?
De: az egyszerű, rövid ismétlések nem mindig probléma – néha a DRY túlerőltetése ront az olvashatóságon.

Miért számít: A duplikált kód azt jelenti, hogy egy bugot minimum két helyen kell javítani – és az egyiket el fogják felejteni.

8. Performance szempontok (csak ha releváns)

Mit nézz: Okoz-e a kód nyilvánvaló teljesítményproblémát?

Kérdések:

Van-e N+1 query probléma? (Ciklusban adatbázis-hívás)
Tölt-e be feleslegesen nagy adatmennyiséget memóriába?
Végez-e szükségtelenül ismétlődő számításokat ciklusban?

# Rossz: N+1 query
orders = Order.all()
for order in orders:
    print(order.user.name)  # Minden iterációban külön DB lekérdezés!

# Jobb: eager loading
orders = Order.includes('user').all()
for order in orders:
    print(order.user.name)  # Egyetlen lekérdezés előre

Fontos: Ne vadássz micro-optimalizációkra. Csak a nyilvánvaló, skálázódó problémákat jelezd.

9. Dokumentáció (ha kell)

Mit nézz: Meg van-e magyarázva, amit a kód neve nem mond el?

Kérdések:

Komplex üzleti logikánál: el van-e magyarázva, MIÉRT ez a megközelítés?
Publikus API vagy könyvtár esetén: van-e dokumentáció a paraméterekről és visszatérési értékről?
Frissítve van-e a meglévő dokumentáció a változással?

Mit NE dokumentálj: Olyat, amit a kód neve már elárul. A # iterate over items komment felesleges egy for item in items sor előtt.

Prioritizálás – nem minden egyformán sürgős

Kategória	Szint	Mit jelent
Biztonság	🔴 Blocker	Merge tilos javítás nélkül
Korrektség / hibakezelés	🔴 Blocker	Merge tilos javítás nélkül
Tesztek hiánya (funkciónál)	🟡 Major	Elvárás, de csapatonként eltérő
Olvashatóság / naming	🟡 Major	Javasolt javítani, de ritkán blokkol
Performance (nyilvánvaló)	🟡 Major	Skálázódó probléma = blocker
DRY / duplikáció	🟢 Minor / Nit	Jelzed, de nem blokkol
Dokumentáció stílus	🟢 Nit	Opcionális visszajelzés

Életszerű példák

Szituáció 1: Az első igazi review

Képzeld el: rád bízzák, hogy review-ld egy másik junior PR-ját. A kód működik, átmegy a teszteken. Mit nézz?

Haladj a listán: biztonsági szempontból nem kerül-e hardcoded token a kódba? Van-e input validáció, ha a felhasználótól vesz adatot? Miután megvagyok ezekkel, nézhetek naming-et, olvashatóságot. Ha csak stílusproblémát találtam, jelzem nit-ként – nem blokkolom.

Szituáció 2: Saját PR előtt

Mielőtt review-ra adod a saját PR-odat, futtasd végig magadon a listát. Ez csökkenti a review körök számát és a review időt.

A leggyakoribb önellenőrzési hiba: elfelejtjük az edge case-eket, mert mi tudjuk, hogyan fog hívódni a kód. A reviewer nem tudja.

Szituáció 3: Sok megjegyzés érkezett – mi blokkol?

Ha 15 kommentet kaptál, ne ess pánikba. Nézd meg a prioritásokat:

Van-e blocker? (Biztonsági hiba, logikai hiba) → Ezek jönnek elsőnek.
Major megjegyzések: javítsd őket, és kérj re-review-t.
Nit-ek: javítsd, ha könnyű, de nem szükséges egyenként reagálni.

Tesztfeladatok

1. feladat – Kategória azonosítás

A következő kód melyik ellenőrzőlista-kategóriába esik a probléma?

def send_email(to, subject, body):
    try:
        smtp.send(to, subject, body)
    except:
        pass

a) Naming konvenciók
b) Hibakezelés
c) Biztonság
d) Duplikált kód

Helyes válasz: b) Hibakezelés – a hiba elnyelődik, semmi sem jelzi, hogy az email küldése sikertelen volt.

2. feladat – Prioritás besorolás

Melyik probléma a legsúlyosabb (blocker)?

// A) Változónév
let x = getUserFromDB(id);

// B) SQL injection
const q = `SELECT * FROM users WHERE id = ${req.params.id}`;

// C) Hiányzó teszt az új helper függvényhez

// D) Duplikált validációs logika két fájlban

a) A
b) B
c) C
d) D

Helyes válasz: b) B – SQL injection biztonsági hiba, blocker. A többit jelezni kell, de nem blokkolja a merge-t.

3. feladat – Edge case felismerés

Mi hiányzik ebből a kódból?

function get_user_age(array $user): int {
    return $user['birthdate']
        ? (int) date_diff(new DateTime($user['birthdate']), new DateTime())->y
        : 0;
}

a) A naming nem megfelelő
b) Nincs kezelve, ha birthdate érvénytelen dátumformátum
c) Nincs return type hint
d) Hiányzik a dokumentáció

Helyes válasz: b) – Érvénytelen dátumformátum esetén new DateTime() kivételt dob, amit nem kezel a kód. Ez korrektség/hibakezelés probléma.

4. feladat – Naming értékelés

Melyik a legjobb függvénynév?

a) handleData()
b) processUserOrderAndSendEmailAndUpdateInventory()
c) validateOrderItems()
d) doStuff()

Helyes válasz: c) – Konkrét, igével kezdődik, egy dolgot csinál. A b) több felelősséget jelez – ezt refaktorálni kellene.

5. feladat – N+1 query azonosítás

Melyik kódrészlet okoz N+1 query problémát?

# A)
users = User.objects.prefetch_related('orders').all()
for user in users:
    print(user.orders.count())

# B)
users = User.objects.all()
for user in users:
    print(user.orders.count())

a) Csak A
b) Csak B
c) Mindkettő
d) Egyik sem

Helyes válasz: b) B – minden iterációban külön query fut orders.count()-hoz. Az A verzió prefetch_related-del előre betölti a kapcsolatot.

6. feladat – Szükséges-e a komment?

# Iterate over all users in the list
for user in users:
    send_welcome_email(user)

a) Igen, segíti az olvashatóságot
b) Nem, a kód neve már elmondja, mi történik
c) Igen, mert a send_welcome_email neve nem egyértelmű
d) Nem, de a függvénynevet kellene javítani

Helyes válasz: b) – A komment szó szerint leírja, amit a kód neve és struktúrája már elárul. Felesleges.

7. feladat – Review prioritás döntés

PR-ban a következő problémákat találtad. Melyiket jelöld blockernek?

a) A logban megjelenik a felhasználó jelszava cleartext-ben
b) Egy változónév temp helyett temporaryUserData lehetne
c) Egy helper függvény nincs tesztelve
d) A kód 3 helyen ismétli ugyanazt az email-validációs regex-et

Helyes válasz: a) – Szenzitív adat a logban biztonsági hiba, azonnal blokkoló. A többi major vagy nit szintű.

8. feladat – Hibakezelés értékelés

Melyik a helyes megközelítés?

// A)
async function fetchUser(id) {
    try {
        return await api.getUser(id);
    } catch (error) {
        console.error('Error fetching user:', error);
        throw new UserNotFoundError(`User ${id} could not be retrieved`);
    }
}

// B)
async function fetchUser(id) {
    return await api.getUser(id);
}

// C)
async function fetchUser(id) {
    try {
        return await api.getUser(id);
    } catch (error) {
        return null;
    }
}

a) A
b) B
c) C
d) Kontextustól függ

Helyes válasz: d) – De A a legjobb általánosan: logol, és értelmes hibát dob felfelé. B nem kezeli a hibát. C elnyeli a hibát és null-t ad vissza, ami a hívónál néma hibát okozhat.

9. feladat – Mi hiányzik a review-ból?

Senior kolléga kap egy PR-t, lefuttatja a teszteket (mind zöld), megnézi a naming-et (rendben), és approve-olja. Mit felejtett el ellenőrizni?

a) Dokumentáció meglétét
b) Biztonságot és input validációt
c) Commit üzenet formátumát
d) Kódstílust a csapat eslint/flake8 konfigurációjához képest

Helyes válasz: b) – A tesztek és naming ellenőrzése nem helyettesíti a biztonsági ellenőrzést. A zöld teszt nem garantálja, hogy nincs SQL injection vagy autentikáció bypass.

10. feladat – Komplex összefoglalás

Az alábbi kódrészletben hány különböző kategóriájú problémát tudsz azonosítani?

def process(d):
    try:
        r = db.execute("SELECT * FROM orders WHERE user_id = " + d['id'])
        for i in r:
            if i['status'] == 'pending':
                if i['amount'] > 0:
                    if i['created_at']:
                        send_mail(i['email'], "Your order is pending")
    except:
        pass
    return r

Azonosítsd a problémákat és kategorizáld őket!

Megoldás:

Biztonság (Blocker): SQL injection – string konkatenáció paraméteres lekérdezés helyett
Hibakezelés (Blocker): Üres except: pass – a hiba elnyelődik, semmit sem tudunk meg
Naming (Major): d, r, i – értelmetlen változónevek
Olvashatóság (Major): 3-szorosan egymásba ágyazott if – early return-nel egyszerűsíthető
Korrektség (Major): r változó a return-nél undefined lehet, ha kivétel keletkezik a db.execute előtt