Gunnar Peipman's ASP.NET blog

ASP.NET, C#, SharePoint, SQL Server and general software development topics.

Sponsors

News

 
 
 
 
 
DZone MVB

Links

Social

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 ?. 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.


kick it on DotNetKicks.com pimp it Shout it

Comments

AndrewSeven said:

Why not reduce the scope of  IList<Invoice> 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 ;)

# March 9, 2009 3:31 PM

anon said:

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

# March 10, 2009 2:49 AM

Mark Needham said:

Agree with AndrewSeven, could have a method signature of Import(IList<Invoice> invoices) unless you do other stuff with the invoices in which case it's fine.

I think that has maybe happened as a result of the class name being an agent noun - it's describing a role rather than describing something which carries out different roles.

I know that's not the point of this post and the refactoring part of it is really good but there's a nice post about the naming stuff here - jupitermoonbeam.blogspot.com/.../agent-nouns-are-code-smells.html

# March 10, 2009 8:29 AM

DigiMortal said:

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.

# March 10, 2009 8:45 AM

Gunnar Peipman's ASP.NET blog said:

Here you can find links to my postings that introduce source code refactoring methods . If I have done

# March 10, 2009 9:10 AM

Refactoring: reduce variable scope - Gunnar Peipman's ASP.NET blog said:

Pingback from  Refactoring: reduce variable scope - Gunnar Peipman&#39;s ASP.NET blog

# March 11, 2009 12:33 AM

DotNetBurner - Tips & Tricks said:

DotNetBurner - burning hot .net content

# June 6, 2009 7:01 AM

PimpThisBlog.com said:

Thank you for submitting this cool story - Trackback from PimpThisBlog.com

# June 6, 2009 7:02 AM

DotNetKicks.com said:

You've been kicked (a good thing) - Trackback from DotNetKicks.com

# June 6, 2009 7:03 AM
Leave a Comment

(required) 

(required) 

(optional)

(required)