3 sure signs your code needs refactoring

In this video, I talk about 3 patterns that often indicates that code could benefit from being refactored.

https://youtu.be/rmdUXtGqYw8

In this video, Erik walks through three telltale signs that your AL code in Business Central needs refactoring. All three revolve around a common theme: stop reusing variables. If you spot any of these patterns in your code, it’s a strong indicator that your code structure could be improved — typically by introducing new procedures and local variables instead of relying on globals and clearing state.

The Three Signs Your Code Needs Refactoring

Erik presents three specific AL commands that, while not inherently wrong, serve as red flags that your code design could be better:

  1. Clear — clearing a record (or JSON/XML object) variable
  2. Reset — removing all filters from a record variable
  3. SetRange with no value — removing a filter from a specific field

Each of these commands often appears because a variable is being reused across multiple logical operations, rather than being scoped properly as a local variable within a dedicated procedure.

Sign #1: The Clear Command

Erik examines a real example from the Business Central base app — specifically the Job Calculate Batches codeunit, inside a function called PostDiffBuffer. The code follows a very typical pattern:

  • A loop iterates over a buffer of records
  • Inside the loop, a Clear is called on a Job Journal Line variable
  • Fields are populated and the line is inserted
  • The loop continues to the next iteration

The Clear exists because the variable is reused across loop iterations. If a conditional block (like validating a work type only for resource lines) sets a value on one iteration, that value could carry over to the next iteration if the variable isn’t cleared. The Clear is a band-aid for a structural problem.

The fix: Extract the inner logic into its own procedure. The Job Journal Line variable becomes a local variable in the new procedure, scoped to a single iteration. There’s no need to clear it because it’s freshly created each time the procedure is called.

As Erik puts it: “The clear itself is not the issue necessarily, but the clear is an indicator that the design is probably not right and the code should be refactored.”

A Historical Note on Globals

Erik points out that in many cases — especially in older code — these variables aren’t even local; they’re global variables used throughout the codeunit. Way back in the early history of Business Central (then NAV), only global variables existed, and adding a new variable required navigating a cumbersome dialog. That’s no longer the case in AL, so there’s no excuse for continuing the pattern.

Sign #2: The Reset Command

The Reset command removes all filters from a record variable. Erik asks the key question: why would you ever want to remove a filter from a variable? In most cases, it’s because the variable has been used somewhere else before, with different filters applied.

Looking at an example from the Post Prepaid Contract Entries report, Erik finds a global record variable used in a PostJournalLine function. The function begins with a Reset to ensure no leftover filters from a previous use. But upon inspection, the variable is only used in that one piece of code — making the global variable (and the associated reset) entirely unnecessary.

The fix: Convert the global to a local variable. If it’s local and freshly declared, there are no stale filters to worry about, and no Reset needed.

Sign #3: SetRange Without a Value (Removing Field Filters)

The third and perhaps most complex sign is using SetRange with no value parameter to remove a filter from a specific field. Erik walks through a particularly convoluted example from a service order page where a global ServiceOrderLine record goes through an elaborate sequence:

  1. Reset the variable (it’s global, so filters may be lingering)
  2. Set filters on Document Type, Document Number, and Quantity Invoiced ≠ 0
  3. If records are found, remove the Quantity Invoiced filter
  4. Add a filter on Outstanding Quantity
  5. If no records found, remove the Outstanding Quantity filter
  6. Add a filter on Quantity Shipped Not Invoiced
  7. Eventually lock the table and query again
  8. Finally, reset everything and go back to finding lines to delete

This “filter on, filter off, wax on, wax off” approach is extremely difficult to read, easy to break, and a maintenance nightmare. If a developer needs to add a field or change a condition, they might forget to reset a filter further down, causing unexpected behavior.

The fix: Use multiple, well-named variables instead of toggling filters on a single variable. For example:

  • ServiceOrderLinesInvoiced — filtered to lines with invoiced quantities
  • ServiceOrderLinesOutstanding — filtered to lines with outstanding quantities
  • ServiceOrderLinesShippedNotInvoiced — filtered to shipped but not invoiced lines

With descriptive variable names, the code becomes self-documenting. You can read what it’s doing without mentally tracking which filters are currently active.

A Code Example to Consider

While Erik’s examples come from the base app, here’s a sample AL file that demonstrates how proper procedure extraction and local scoping should look in practice:

pageextension 50100 CustomerListExt extends "Customer List"
{
    trigger OnOpenPage();
    begin
        Message(GenerateAwesomeMessage(TODAY()));
    end;

    local procedure GenerateAwesomeMessage(T: Date): Text
    var
        d: Integer;
        m: Integer;
        y: Integer;
    begin
        d := 3;
        m := 4;
        y := 5;
        newProcedure(d, m);
    end;

    local procedure newProcedure(var d: Integer; var m: Integer): Text
    begin
        for d := m to m * 5 do begin
            for m := d to d * 5 do begin
                if d = m then
                    exit('Test!!!');
            end;
        end;
    end;
}

Even in this small example, you can see variables being passed by reference (var parameters) and mutated inside loops — patterns that can quickly become confusing. In production code, each logical unit of work should ideally have its own procedure with clearly scoped local variables.

Summary

The core message is simple: stop reusing variables. When you see Clear, Reset, or SetRange with no value in your AL code, treat them as warning signs. They often indicate that:

  • A variable is being shared across multiple logical operations when it shouldn’t be
  • Code that should be extracted into a separate procedure hasn’t been
  • Global variables are being used where local variables would be safer and cleaner

The fix is almost always the same: extract the logic into its own procedure, use local variables, and give those variables descriptive names that communicate intent. Your code will be easier to read, easier to maintain, and far less prone to subtle bugs caused by leftover state.

Picture of hougaard

hougaard