Delphi-spec-kit Delphi Code Review

Delphi code review checklist — quality, security, performance, SOLID, memory

install
source · Clone the upstream repo
git clone https://github.com/delphicleancode/delphi-spec-kit
Claude Code · Install into ~/.claude/skills/
T=$(mktemp -d) && git clone --depth=1 https://github.com/delphicleancode/delphi-spec-kit "$T" && mkdir -p ~/.claude/skills && cp -r "$T/.gemini/skills/code-review" ~/.claude/skills/delphicleancode-delphi-spec-kit-delphi-code-review-94bc88 && rm -rf "$T"
manifest: .gemini/skills/code-review/SKILL.md
source content

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

// ❌ 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

PrincipleCheck
SRPDoes class have ONE responsibility? Service does not access data?
OCPDo new features add classes, not modify existing ones?
LSPDoes either implementation of the interface work in place of the other?
ISPDoesn't interface have methods that implementers don't use?
DIPConstructor takes interfaces, not concrete classes?