Delphi Code Review — Skill
Quick Checklist
Corretude
Security
Performance
Code Quality
Memory Management
Pascal Nomenclature
Tests
Documentation
Anti-Patterns to Flag
// ❌ Números mágicos
if ACustomer.Age > 18 then
// ✅ Constantes nomeadas
const MINIMUM_AGE = 18;
if ACustomer.Age > MINIMUM_AGE then
// ❌ with statement
with AQuery do begin
SQL.Text := '...';
Open;
end;
// ✅ Referência explícita
AQuery.SQL.Text := '...';
AQuery.Open;
// ❌ Catch genérico
except
on E: Exception do ShowMessage(E.Message);
// ✅ Exceptions específicas
except
on E: EFDDBEngineException do
raise EDatabaseException.Create('Falha: ' + E.Message);
// ❌ Logic em OnClick
procedure TfrmMain.btnSaveClick(Sender: TObject);
begin
// 50 linhas de logic de negócio aqui
end;
// ✅ Delegar para Service
procedure TfrmMain.btnSaveClick(Sender: TObject);
begin
FService.SaveCustomer(GetFormData);
end;
// ❌ Memory leak
function GetItems: TStringList;
begin
Result := TStringList.Create;
LoadItems(Result); // se LoadItems lançar exception, leak!
end;
// ✅ Seguro
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? |
