X

Refactoring: reduce variable scope

In good code variables are used as short as possible. Often we can see code where variables are defined in wider scope than it is necessary. There are many examples about too wide scopes. One of fuzziest of them is variable that is defined in class scope but it used only by one method and this method uses this variable as local variable. But variable life time can also be reduced in local scope. To achieve this we use refactoring method called reduce variable scope.

Let’s look at the following method.

public class InvoiceImporter
{
    private readonly IList<Invoice> _invoices;

    public InvoiceImporter(IList<Invoice> invoices)
    {
        _invoices = invoices;
    }

    public void Import()
    {
        int i;
        Invoice invoice;

        Console.WriteLine("Starting invoices import");

        if (_invoices.Count == 0)
        {
            Console.WriteLine("No invoices to import.");
            return;
        }

        Console.WriteLine("Invoices to import: " + _invoices.Count);

        for (i = 0; i < _invoices.Count; i++)
        {
            invoice = _invoices[i];
            Console.WriteLine("Importing invoice " + invoice.InvoiceNo);
            ImportInvoice(invoice);
        }

        Console.WriteLine("Import done!");
    }

    private void ImportInvoice(Invoice invoice)
    {
        // Importing logic here
    }

    // do something with _invoices
}

We can see some variable defined in the beginning of method. One of them – counter – is used as for loop counter. Next suspicious variable is invoice. It is used only inside for loop and therefore we have no reason to keep it available in full method scope. After changing the scope of these two variables our Import() looks like this.

public void Import()
{
    Console.WriteLine("Starting invoices import");

    if (_invoices.Count == 0)
    {
        Console.WriteLine("No invoices to import.");
        return;
    }

    Console.WriteLine("Invoices to import: " + _invoices.Count);

    for (int i = 0; i < _invoices.Count; i++)
    {
        var invoice = _invoices[i];
        Console.WriteLine("Importing invoice " + invoice.InvoiceNo);
        ImportInvoice(invoice);
    }

    Console.WriteLine("Import done!");
}

Now we have these two variables in the scope they should have been at first place. They cannot confuse other developers and also it is not possible to use these variables outside the for loop.

Liked this post? Empower your friends by sharing it!
Categories: C#

View Comments (3)

  • Why not reduce the scope of IList too?

    If it isn't used by other parts of the class, it could be a parameter of the Import method.

    The code also looks like it would be better served by a foreach loop ;)

  • I would be interested to see what the Visual Studio code analysis thinks is the correct scope for the invoice variable?

  • To make my point better understandable I added comment // do something with _invoices to the first code block.

    _invoices is class attribute that is used also by other methods. I hope this comment makes things more clear.

Related Post