Delphi Code Refactoring

Delphi Code Refactoring — Skill

Use this skill when the user requests refactoring, code review or removal of code smells in Object Pascal. Refactoring never changes observable behavior — it only improves the internal structure of the code.

When to Use

• User asks to "improve", "clean up" or "refactor" existing code
• When reviewing code and finding code smells
• When preparing legacy code to receive new functionality
• Before adding tests to untestable code
• When answering "why is this code difficult to understand?"

Fundamental Principle

> "Refactoring is the art of changing the structure of code without changing its behavior." > Always write (or check for) tests before refactoring.

---

📋 Catalog of Code Smells and Techniques

1. Extract Method — Long Method

Detect: Method with more than 20 lines or that needs comments to explain blocks.

Before:

procedure TInvoiceService.GenerateInvoice(AOrder: TOrder);
var
  LTax, LSubtotal, LTotal: Currency;
  LItem: TOrderItem;
  LLines: TStringList;
begin
  // Calcula subtotal
  LSubtotal := 0;
  for LItem in AOrder.Items do
    LSubtotal += LItem.UnitPrice * LItem.Quantity;

  // Calcula imposto
  if AOrder.IsExempt then LTax := 0
  else LTax := LSubtotal * 0.12;

  LTotal := LSubtotal + LTax;

  // Gera linhas do relatório
  LLines := TStringList.Create;
  try
    LLines.Add('NOTA FISCAL');
    LLines.Add(Format('Cliente: %s', [AOrder.Customer.Name]));
    for LItem in AOrder.Items do
      LLines.Add(Format('  %s x%d = R$%.2f',
        [LItem.Product.Name, LItem.Quantity, LItem.UnitPrice * LItem.Quantity]));
    LLines.Add(Format('Total: R$%.2f', [LTotal]));
    LLines.SaveToFile(AOrder.InvoicePath);
  finally
    LLines.Free;
  end;
end;

After:

procedure TInvoiceService.GenerateInvoice(AOrder: TOrder);
var
  LSubtotal, LTax: Currency;
begin
  LSubtotal := CalculateSubtotal(AOrder);
  LTax      := CalculateTax(AOrder, LSubtotal);
  SaveInvoiceFile(AOrder, LSubtotal + LTax);
end;

function TInvoiceService.CalculateSubtotal(AOrder: TOrder): Currency;
var LItem: TOrderItem;
begin
  Result := 0;
  for LItem in AOrder.Items do
    Result := Result + (LItem.UnitPrice * LItem.Quantity);
end;

function TInvoiceService.CalculateTax(AOrder: TOrder; ASubtotal: Currency): Currency;
const
  STANDARD_TAX_RATE = 0.12;
begin
  if AOrder.IsExempt then Result := 0
  else Result := ASubtotal * STANDARD_TAX_RATE;
end;

procedure TInvoiceService.SaveInvoiceFile(AOrder: TOrder; ATotal: Currency);
var
  LLines: TStringList;
  LItem: TOrderItem;
begin
  LLines := TStringList.Create;
  try
    LLines.Add('NOTA FISCAL');
    LLines.Add(Format('Cliente: %s', [AOrder.Customer.Name]));
    for LItem in AOrder.Items do
      LLines.Add(Format('  %s x%d = R$%.2f',
        [LItem.Product.Name, LItem.Quantity, LItem.UnitPrice * LItem.Quantity]));
    LLines.Add(Format('Total: R$%.2f', [ATotal]));
    LLines.SaveToFile(AOrder.InvoicePath);
  finally
    LLines.Free;
  end;
end;

---

2. Extract Class — Class with Multiple Responsibilities

Detect: Class with fields of different nature, methods without cohesion.

Before:

TEmployee = class
private
  // Data pessoais
  FName: string;
  FBirthDate: TDate;
  FCpf: string;
  // Data salariais
  FBaseSalary: Currency;
  FBonusPercentage: Double;
  FDepartmentId: Integer;
  // Data de RH
  FHiredDate: TDate;
  FVacationDaysLeft: Integer;
  FPerformanceScore: Integer;
public
  function CalculateGrossSalary: Currency;
  function CalculateNetSalary: Currency;
  function CalculateVacationPay: Currency;
  function GetNextVacationDate: TDate;
  function GetYearsOfService: Integer;
  function IsEligibleForBonus: Boolean;
end;

After:

// Value Object — imutável
TEmployeePersonalData = record
  Name: string;
  BirthDate: TDate;
  Cpf: string;
end;

// Classe coesa: responsibility salarial
TEmployeeSalary = class
private
  FBaseSalary: Currency;
  FBonusPercentage: Double;
public
  constructor Create(ABaseSalary: Currency; ABonusPercentage: Double);
  function CalculateGross: Currency;
  function CalculateNet: Currency;
  function IsEligibleForBonus: Boolean;
end;

// Classe coesa: responsibility de RH
TEmployeeHrRecord = class
private
  FHiredDate: TDate;
  FVacationDaysLeft: Integer;
  FPerformanceScore: Integer;
public
  function GetYearsOfService: Integer;
  function GetNextVacationDate: TDate;
  function CalculateVacationPay(AGrossSalary: Currency): Currency;
end;

// Entidade principal — agora only agrega as partes
TEmployee = class
private
  FPersonalData: TEmployeePersonalData;
  FSalary: TEmployeeSalary;
  FHrRecord: TEmployeeHrRecord;
public
  constructor Create(AData: TEmployeePersonalData;
    ASalary: TEmployeeSalary; AHr: TEmployeeHrRecord);
  destructor Destroy; override;
  property PersonalData: TEmployeePersonalData read FPersonalData;
  property Salary: TEmployeeSalary read FSalary;
  property HrRecord: TEmployeeHrRecord read FHrRecord;
end;

---

3. Replace Nested Conditionals with Guard Clauses

Detect: More than 2 levels of if..then..begin..end nested.

Before:

function TBankService.Withdraw(AAccount: TAccount; AAmount: Currency): Boolean;
begin
  Result := False;
  if Assigned(AAccount) then
  begin
    if AAccount.IsActive then
    begin
      if AAmount > 0 then
      begin
        if AAccount.Balance >= AAmount then
        begin
          if not AAccount.IsBlocked then
          begin
            AAccount.Balance := AAccount.Balance - AAmount;
            FRepository.Save(AAccount);
            Result := True;
          end;
        end;
      end;
    end;
  end;
end;

After:

function TBankService.Withdraw(AAccount: TAccount; AAmount: Currency): Boolean;
begin
  if not Assigned(AAccount) then
    raise EArgumentNilException.Create('Conta não pode ser nula');
  if not AAccount.IsActive then
    raise EBusinessRuleException.Create('Conta inativa');
  if AAmount = 3 then LockAccount(AUser);
LInterest := ADebt * 0.02;  // juros de mora

After:

const
  LOW_STOCK_THRESHOLD       = 5;
  MAX_INSTALLMENTS          = 12;
  MIN_PASSWORD_LENGTH       = 8;
  MAX_FAILED_LOGIN_ATTEMPTS = 3;
  MONTHLY_INTEREST_RATE     = 0.02;

if AProduct.Stock = MAX_FAILED_LOGIN_ATTEMPTS then LockAccount(AUser);
LInterest := ADebt * MONTHLY_INTEREST_RATE;

---

5. Replace Conditional with Polymorphism (Strategy/State)

Detect: case or if/else if string that checks the type or regime of an object.

Before:

function TShippingService.CalculateFreight(AOrder: TOrder): Currency;
begin
  case AOrder.ShippingMethod of
    smPAC:     Result := AOrder.Weight * 2.50 + 8.0;
    smSEDEX:   Result := AOrder.Weight * 4.80 + 15.0;
    smTransp:  Result := AOrder.Weight * 1.20;
    smRetirada:Result := 0;
  end;
end;

After:

// Interface Strategy
IFreightStrategy = interface
  ['{GUID}']
  function Calculate(AWeightKg: Double): Currency;
  function GetCarrierName: string;
end;

// Implementations: uma por variaction
TPACFreightStrategy = class(TInterfacedObject, IFreightStrategy)
  function Calculate(AWeightKg: Double): Currency;   // peso * 2.50 + 8.0
  function GetCarrierName: string;
end;

TSEDEXFreightStrategy = class(TInterfacedObject, IFreightStrategy)
  function Calculate(AWeightKg: Double): Currency;   // peso * 4.80 + 15.0
  function GetCarrierName: string;
end;

TTransportadoraFreightStrategy = class(TInterfacedObject, IFreightStrategy)
  function Calculate(AWeightKg: Double): Currency;   // peso * 1.20
  function GetCarrierName: string;
end;

TRetiradaFreightStrategy = class(TInterfacedObject, IFreightStrategy)
  function Calculate(AWeightKg: Double): Currency;   // sempre 0
  function GetCarrierName: string;
end;

// Context — not precisa mais do case
function TShippingService.CalculateFreight(AOrder: TOrder;
  AStrategy: IFreightStrategy): Currency;
begin
  Result := AStrategy.Calculate(AOrder.WeightKg);
end;

---

6. Introduce Parameter Object

Detect: Method with > 4 parameters, especially if several are optional.

Before:

function TReportService.GenerateSalesReport(
  AStartDate: TDate;
  AEndDate: TDate;
  const AProductCategory: string;
  const ASalesRepId: string;
  AMinAmount: Currency;
  AGroupByMonth: Boolean;
  AIncludeReturns: Boolean): TReportResult;

After:

TSalesReportFilter = record
  StartDate: TDate;
  EndDate: TDate;
  ProductCategory: string;
  SalesRepId: string;
  MinAmount: Currency;
  GroupByMonth: Boolean;
  IncludeReturns: Boolean;
  // Construtor com defaults
  class function Default: TSalesReportFilter; static;
end;

class function TSalesReportFilter.Default: TSalesReportFilter;
begin
  Result.StartDate       := StartOfYear(Date);
  Result.EndDate         := Date;
  Result.ProductCategory := '';
  Result.SalesRepId      := '';
  Result.MinAmount       := 0;
  Result.GroupByMonth    := False;
  Result.IncludeReturns  := False;
end;

function TReportService.GenerateSalesReport(
  AFilter: TSalesReportFilter): TReportResult;

---

7. Remove with Statement

Detect: Any with Objeto do begin...end in the code.

Rule: Never use with. Prefer local variables or explicit qualification.

Before:

procedure TCustomerForm.LoadData;
begin
  with qryCustomers do
  begin
    Close;
    SQL.Text := 'SELECT * FROM customers WHERE id = :id';
    ParamByName('id').AsInteger := FCustomerId;
    Open;
    with FieldByName('address') do
    begin
      edtStreet.Text := AsString;
    end;
  end;
end;

After:

procedure TCustomerForm.LoadData;
var LAddressField: TField;
begin
  qryCustomers.Close;
  qryCustomers.SQL.Text := 'SELECT * FROM customers WHERE id = :id';
  qryCustomers.ParamByName('id').AsInteger := FCustomerId;
  qryCustomers.Open;

  LAddressField := qryCustomers.FieldByName('address');
  edtStreet.Text := LAddressField.AsString;
end;

---

8. Extract Interface / Invert Dependency

Detect: Service or class depending directly on concrete class (not interface).

Before:

TOrderService = class
private
  FEmailSender: TSmtpEmailSender;      // classe concreta
  FRepository:  TFireDACOrderRepo;     // classe concreta
public
  constructor Create;  // instancia internamente — impossível testar
end;

constructor TOrderService.Create;
begin
  FEmailSender := TSmtpEmailSender.Create('smtp.server.com', 587);
  FRepository  := TFireDACOrderRepo.Create(GetDatabaseConnection);
end;

After:

// 1. Extraia interfaces
IEmailSender = interface
  ['{GUID}']
  procedure Send(const ATo, ASubject, ABody: string);
end;

IOrderRepository = interface
  ['{GUID}']
  function FindById(AId: Integer): TOrder;
  procedure Save(AOrder: TOrder);
end;

// 2. Injete no construtor — testável e flexível
TOrderService = class
private
  FEmailSender: IEmailSender;
  FRepository:  IOrderRepository;
public
  constructor Create(ARepository: IOrderRepository; AEmailSender: IEmailSender);
end;

// 3. Em testes, injete fakes
TFakeEmailSender = class(TInterfacedObject, IEmailSender)
  FSentMessages: TStringList;
  procedure Send(const ATo, ASubject, ABody: string);
end;

---

9. Rename — Self-Descriptive Names

Detect: Variables like x, tmp, data, flag; methods like Proc1, DoIt, Handle.

// ❌ ANTES
var x, tmp: Integer;
    s: string;
    flag: Boolean;
procedure Handle(d: TData);
function Calc(v: Double): Double;

// ✅ DEPOIS
var LRetryCount, LMaxRetries: Integer;
    LCustomerFullName: string;
    LIsPaymentApproved: Boolean;
procedure ProcessCustomerOrder(AOrderData: TOrderData);
function CalculateShippingCost(AWeightKg: Double): Currency;

---

10. Inline Method — Unnecessarily Delegate Method

Detect: Method that just calls another method, without additional logic.

// ❌ ANTES — delegaction desnecessária
function TOrder.GetTotal: Currency;
begin
  Result := CalculateOrderTotal;  // só delega
end;

function TOrder.CalculateOrderTotal: Currency;
begin
  Result := FSubtotal + FTax - FDiscount;
end;

// ✅ DEPOIS — only um método com nome descritivo
function TOrder.GetTotal: Currency;
begin
  Result := FSubtotal + FTax - FDiscount;
end;

---

🔄 Secure Refactoring Process

1. ENTENDER  → Leia e compreenda o código atual
2. TESTAR    → Escreva/execute testes que cobrem o comportamento atual
3. PEQUENO   → Faça uma refatoração por vez (não tudo de uma vez)
4. TESTAR    → Execute os testes novamente — devem continuar passando
5. COMMIT    → Commit atômico por refatoração (mensagem: "refactor: extract CalculateTax")
6. REPETIR   → Próxima refatoração

✅ Final Checklist

• [ ] Is there testing covering the behavior before refactoring?
• [ ] Does the observable behavior remain the same?
• [ ] No with was introduced?
• [ ] No nameless magic number?
• [ ] Resulting method has ≤ 20 lines?
• [ ] Name of the extracted method needs no comment?
• [ ] Dependencies are now via interface?
• [ ] Do guard clauses replace deep nesting?
• [ ] Were excess parameters grouped in DTO/Record?

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: delphicleancode
• Source: delphicleancode/delphi-spec-kit
• License: MIT
• Homepage: https://inovefast.com.br

Install and usage instructions live in the source repository linked above.