Code Review in JavaScript: Was prüfen und wie man Feedback gibt
Code Review ist nicht nur eine formale Prüfung vor dem Merge eines Pull-Requests. Es ist ein mächtiges Werkzeug zur Verbesserung der Codequalität, zum Wissensaustausch im Team und zur frühzeitigen Vermeidung von Bugs. In der Welt von JavaScript und Node.js, wo Asynchronität und dynamische Typisierung viele versteckte Fallstricke schaffen, wird ein kompetentes Review kritisch wichtig.
In diesem Artikel analysieren wir, worauf man bei der Überprüfung von JavaScript-Code konkret achten sollte und wie man Feedback gibt, das nützlich und nicht verletzend ist.
1. Logik, Architektur und Sicherheit
Das Erste, was überprüft werden muss, ist, ob der Code die gestellte Aufgabe löst. Vertiefen Sie sich nicht sofort in die Formatierung. Stellen Sie sich folgende Fragen:
- Funktioniert die Logik korrekt? Gibt es Randfälle (edge cases)? Was passiert, wenn
null,undefinedoder ein leeres Array übergeben wird? - Gibt es Sicherheitslücken? In Node.js ist dies besonders relevant. Prüfen Sie, ob Rohdaten aus der Anfrage direkt in eine SQL-Abfrage (SQL-Injection) oder in
eval()übergeben werden. Werden Sanitizer für HTML verwendet? - Wird die Architektur eingehalten? Verstößt der Code gegen die SOLID-Prinzipien? Macht eine Funktion beispielsweise zu viel? Ist die Geschäftslogik aus den Route-Handlern ausgelagert?
// SCHLECHT: Vermischung von Logik und HTTP-Schichtapp.get('/user/:id', async (req, res) => { const user = await db.query(`SELECT * FROM users WHERE id = ${req.params.id}`); // SQL Injection! res.json(user);});
// GUT: Trennung der Verantwortlichkeitenconst getUserById = async (id) => { if (!id || typeof id !== 'string') throw new Error('Invalid ID'); return await db.query('SELECT * FROM users WHERE id = $1', [id]);};app.get('/user/:id', async (req, res) => { const user = await getUserById(req.params.id); res.json(user);});2. Asynchronität und Arbeit mit Promises
Dies ist die Achillesferse vieler JavaScript-Entwickler. Bei der Überprüfung von Node.js-Code müssen Sie unbedingt prüfen:
- Fehlerbehandlung. Sind alle asynchronen Funktionen in try/catch-Blöcke eingeschlossen? Fehlt ein
.catch()bei Promises? Gibt es "verwaiste" Promises (unhandled promise rejections)? - Korrekte Verwendung von async/await. Gibt es unnötige
await-Aufrufe, wo Promises parallel mitPromise.allgestartet werden könnten? - Race Conditions. Kann der Code bei parallelen Anfragen an dieselbe Ressource (z. B. eine Datei oder globale Variable) inkorrekt ausgeführt werden?
// SCHLECHT: Sequenzielle Ausführung unabhängiger Anfragenconst user = await getUser(id);const posts = await getPosts(id);
// GUT: Parallele Ausführungconst [user, posts] = await Promise.all([getUser(id), getPosts(id)]);
// SCHLECHT: Vergessene Fehlerbehandlungrouter.get('/data', async (req, res) => { const data = await fetchData(); // Wenn fetchData fehlschlägt, "stürzt" der Server auch ab res.json(data);});
// GUTrouter.get('/data', async (req, res, next) => { try { const data = await fetchData(); res.json(data); } catch (error) { next(error); // Fehler an den globalen Handler übergeben }});3. Codequalität, Stil und Namensgebung
Code sollte lesbar sein wie ein gutes Buch. Selbst wenn die Logik korrekt ist, verlangsamt ein schlechter Stil die zukünftige Entwicklung.
- Benennung von Variablen und Funktionen. Namen sollten die Fragen "Was ist das?" und "Was macht es?" beantworten. Vermeiden Sie Ein-Buchstaben-Variablen (außer für Schleifenzähler) und Abkürzungen wie
getUsrInf. - Magische Zahlen und Zeichenketten. Werden sie in Konstanten ausgelagert?
const MAX_RETRIES = 3;ist viel verständlicher als