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

| Principle | Check | |-----------|-----------| | SRP | Does class have ONE responsibility? Service does not access data? | | OCP | Do new features add classes, not modify existing ones? | | LSP | Does either implementation of the interface work in place of the other? | | ISP | Doesn't interface have methods that implementers don't use? | | DIP | Constructor takes interfaces, not concrete classes? |

Delphi Code Review

Como adicionar

Cole no README do seu repo

Skills relacionadas

internal-comms

smart-explore

babysit

do

Receba novas skills de DevOps e Infra toda segunda