Delphi Code Review

Delphi Code Review — Skill

Quick Checklist

Corretude

• Code does what it's supposed to do
• Edge cases handled (nil, empty list, zero value)
• Error handling implemented with specific exceptions
• No obvious bugs

Security

• Parameterized SQL queries (without string concatenation)
• Validated and sanitized input
• No hardcoded credentials or passwords
• No SQL injection via Format or concatenation in queries

Performance

• No N+1 queries (avoid loop with query inside)
• No unnecessary loops
• Large objects released as early as possible
• TObjectList with OwnsObjects configured correctly

Code Quality

• Self-descriptive names following Pascal Guide
• DRY — no duplicate code
• SOLID — principles respected
• Methods ≤ 20 lines
• Guard clauses instead of deep nesting

Memory Management

• try/finally with Free for temporary objects
• Interfaces for automatic reference counting
• Assigned() before accessing references that may be nil
• Destructor Destroy with override freeing owned fields
• No memory leaks in exception paths

Pascal Nomenclature

• PascalCase for all identifiers
• Prefix T in classes, I in interfaces, E in exceptions
• Prefix F in private fields, A in parameters, L in local variables
• Units: Projeto.Camada.Dominio.Funcionalidade.pas
• Components: 3-letter prefix (btn, edt, lbl, etc.)

Tests

• Unit tests for new code
• Edge cases tested
• Readable and maintainable tests

Documentation

• XMLDoc for public methods and properties
• Comments in Portuguese when necessary
• Do not comment self-explanatory code

Anti-Patterns to Flag

//❌ Magic numbers
if ACustomer.Age > 18 then

//✅ Named constants
const MINIMUM_AGE = 18;
if ACustomer.Age > MINIMUM_AGE then

//❌ with statement
with AQuery do begin
  SQL.Text := '...';
  Open;
end;

//✅ Explicit reference
AQuery.SQL.Text := '...';
AQuery.Open;

//❌ Generic Catch
except
  on E: Exception do ShowMessage(E.Message);

//✅ Specific exceptions
except
  on E: EFDDBEngineException do
    raise EDatabaseException.Create('Falha: ' + E.Message);

//❌ Logic in OnClick
procedure TfrmMain.btnSaveClick(Sender: TObject);
begin
  //50 lines of business logic here
end;

//✅ Delegate for Service
procedure TfrmMain.btnSaveClick(Sender: TObject);
begin
  FService.SaveCustomer(GetFormData);
end;

// ❌ Memory leak
function GetItems: TStringList;
begin
  Result := TStringList.Create;
  LoadItems(Result); //if LoadItems throws exception, leak!
end;

//✅ Safe
function GetItems: TStringList;
begin
  Result := TStringList.Create;
  try
    LoadItems(Result);
  except
    Result.Free;
    raise;
  end;
end;

Review Comments Guide

🔴 BLOQUEANTE: Memory leak — objeto não liberado em caso de exception
🔴 BLOQUEANTE: SQL injection — query usando concatenação de string

🟡 SUGESTÃO: Extrair método — este bloco tem 35 linhas
🟡 SUGESTÃO: Usar interface em vez de classe concreta (DIP)

🟢 NIT: Renomear variável 'S' para nome descritivo
🟢 NIT: Preferir guard clause a nesting

❓ PERGUNTA: O que acontece se ACustomer for nil aqui?
❓ PERGUNTA: Este objeto é liberado por quem?

Specific SOLID Checklist