Code Reviews
Language Recipes
Language-specific code review guidance for TypeScript, Python, C#, Go, and more
Different programming languages have different idioms, patterns, and pitfalls. This guide provides language-specific review guidance to complement the general checklist.
TypeScript / JavaScript
Type Safety
- Avoid
anytype where possible - Use strict null checks
- Prefer interfaces over type aliases for objects
- Use enums or const objects for fixed values
- Generic types are appropriately constrained
// Avoid
function process(data: any) { ... }
// Better
function process(data: UserData) { ... }
// Best (with generics)
function process<T extends BaseData>(data: T) { ... }Async Patterns
- Promises properly awaited
- Error handling in async functions
- No unhandled promise rejections
- Appropriate use of Promise.all vs sequential awaits
- Cancellation handled where needed
// Avoid - unhandled rejection
async function fetchData() {
fetch('/api/data'); // Missing await and error handling
}
// Better
async function fetchData() {
try {
const response = await fetch('/api/data');
if (!response.ok) throw new Error('Failed to fetch');
return await response.json();
} catch (error) {
console.error('Fetch error:', error);
throw error;
}
}React Specific
- Hooks follow rules (top level, consistent order)
- Dependencies array correct in useEffect
- Memoization used appropriately
- Keys are stable and unique
- State updates are immutable
// Avoid - missing dependency
useEffect(() => {
fetchUser(userId);
}, []); // userId should be in deps
// Better
useEffect(() => {
fetchUser(userId);
}, [userId]);Common Issues
- Using
==instead of=== - Mutating state directly
- Memory leaks (event listeners, subscriptions)
- Blocking the main thread
- Not handling loading/error states
Python
Pythonic Code
- Follow PEP 8 style guide
- Use list comprehensions appropriately
- Context managers for resources
- Type hints where helpful
- Docstrings for public APIs
# Avoid
result = []
for item in items:
if item.active:
result.append(item.name)
# Better
result = [item.name for item in items if item.active]Error Handling
- Specific exceptions caught (not bare except)
- Resources cleaned up (try/finally or context managers)
- Errors logged with context
- Custom exceptions where appropriate
# Avoid
try:
process_data(data)
except: # Catches everything including KeyboardInterrupt
pass
# Better
try:
process_data(data)
except ValueError as e:
logger.error(f"Invalid data: {e}")
raisePerformance
- Generators for large sequences
- Appropriate data structures (set for membership, dict for lookup)
- Avoid repeated expensive operations
- Profile before optimizing
# Avoid - creates full list in memory
def get_items():
return [process(x) for x in huge_list]
# Better - generator for memory efficiency
def get_items():
for x in huge_list:
yield process(x)Common Issues
- Mutable default arguments
- Circular imports
- Global state
- Not closing files/connections
- Inefficient string concatenation
C#
Modern C# Features
- Nullable reference types enabled
- Pattern matching where appropriate
- Records for immutable data
- Async/await used correctly
- LINQ used appropriately
// Older style
if (obj is MyType)
{
var typed = (MyType)obj;
typed.DoSomething();
}
// Modern pattern matching
if (obj is MyType typed)
{
typed.DoSomething();
}Async Patterns
- ConfigureAwait(false) in library code
- Avoid async void (except event handlers)
- CancellationToken used where appropriate
- No sync over async (or vice versa)
// Avoid - async void
public async void ProcessData() { ... }
// Better - async Task
public async Task ProcessDataAsync(CancellationToken ct = default)
{
await DoWorkAsync().ConfigureAwait(false);
}Resource Management
- IDisposable implemented correctly
- using statements for disposables
- async using for async disposables
- Finalizers only when needed
// Avoid
var connection = new SqlConnection(connString);
// ... use connection
connection.Dispose();
// Better
using var connection = new SqlConnection(connString);
// ... connection disposed automaticallyCommon Issues
- Null reference exceptions
- Deadlocks from blocking async
- Memory leaks (event handlers)
- Boxing/unboxing overhead
- String concatenation in loops
Go
Go Idioms
- Error handling follows conventions
- Interfaces are small and focused
- Goroutines properly managed
- Channels used correctly
- defer used appropriately
// Avoid - ignoring errors
file, _ := os.Open(filename)
// Better
file, err := os.Open(filename)
if err != nil {
return fmt.Errorf("failed to open file: %w", err)
}
defer file.Close()Concurrency
- No data races (use race detector)
- Goroutines have exit conditions
- Context used for cancellation
- Channels closed by sender
- Select with default for non-blocking
// Avoid - potential goroutine leak
go func() {
for item := range ch {
process(item)
}
}()
// Better - with context
go func(ctx context.Context) {
for {
select {
case item := <-ch:
process(item)
case <-ctx.Done():
return
}
}
}(ctx)Error Handling
- Errors wrapped with context
- Custom error types where needed
- errors.Is/As for comparison
- Don't ignore errors
// Avoid
if err != nil {
return err
}
// Better - add context
if err != nil {
return fmt.Errorf("failed to process user %s: %w", userID, err)
}Common Issues
- Nil pointer dereference
- Race conditions
- Goroutine leaks
- Not checking errors
- Misusing slices (append gotchas)
Java
Modern Java
- Use var where it improves readability
- Streams used appropriately
- Optional instead of null returns
- Records for data classes
- Switch expressions
// Older style
String result;
switch (status) {
case ACTIVE: result = "Active"; break;
case INACTIVE: result = "Inactive"; break;
default: result = "Unknown";
}
// Modern switch expression
var result = switch (status) {
case ACTIVE -> "Active";
case INACTIVE -> "Inactive";
default -> "Unknown";
};Null Safety
- Use Optional for nullable returns
- Check nulls at boundaries
- Use @Nullable/@NonNull annotations
- Avoid returning null collections
// Avoid
public User findUser(String id) {
return userMap.get(id); // might return null
}
// Better
public Optional<User> findUser(String id) {
return Optional.ofNullable(userMap.get(id));
}Common Issues
- NullPointerException
- Resource leaks (try-with-resources)
- Mutable objects as map keys
- Checked exception overuse
- StringBuilder vs string concatenation
Bash/Shell Scripts
Safety
- set -euo pipefail at the start
- Variables quoted properly
- Input validated
- Temporary files cleaned up
- Portable syntax used
#!/bin/bash
set -euo pipefail
# Avoid
rm -rf $DIR/* # Dangerous if DIR is empty
# Better
rm -rf "${DIR:?}"/* # Fails if DIR is unsetBest Practices
- Functions for reusable code
- Meaningful variable names
- Error messages to stderr
- Exit codes used correctly
- ShellCheck passes
# Avoid
if [ $? -eq 0 ]; then
# Better
if command_that_might_fail; thenYAML (Azure Pipelines, GitHub Actions)
Structure
- Indentation consistent (2 spaces)
- Anchors used to avoid duplication
- Secrets not hardcoded
- Conditions are clear
- Jobs have meaningful names
# Avoid - hardcoded values
- script: |
curl -u admin:password123 https://api.example.com
# Better - use secrets
- script: |
curl -u ${{ secrets.API_USER }}:${{ secrets.API_TOKEN }} https://api.example.comSecurity
- Secrets managed properly
- Least privilege for service accounts
- Dependencies pinned to versions
- Third-party actions reviewed
Related Resources
Compliance
This section fulfills ISO 13485 requirements for design outputs (7.3.4) and production control (7.5.1), and ISO 27001 requirements for secure coding (A.8.28) and secure development lifecycle (A.8.25). Language-specific patterns cover type safety, resource management, error handling, and secrets management.
How is this guide?
Last updated on