Refactoring: delete unneeded code

It is not a miracle that during coding developers produce code that is later not needed anymore. It is also not a miracle that unneeded code will stay in project files waiting for the day when somebody finds this code useful (again). But this far this code is just sitting there and causing problems to people who look at these modules first time.

Let’s look at the following code.

public class Invoice 
{ 
    private IList<InvoiceLine> _lines;  
 
    // ... 
 
    public decimal Sum() 
    { 
        decimal sum = 0; 
 
        foreach(InvoiceLine line in _lines) 
        { 
            decimal lineSum = line.CalculateTotal(); 
            // lineSum = lineSum * (1 - lineSum.Discount) 
            // lineSum = lineSum * 1.12 
            // lineSum = lineSum * 1.01 
            sum += lineSum; 
        } 
 
        // sum = sum * 1.12 
        // sum = sum * 1.01 
 
        return sum; 
    } 
 
    public void ValidateInvoice() 
    { 
        if(_lines.Count == 0) 
            throw new Exception("Invoice body cannot be empty!"); 
        if(_lines == null) 
            throw new Exception("Invoice lines collection is null"); 
    } 
}

This code has some things I don’t like. And I think these things doesn’t like also to other developers who have to work with this code.

• The code is hard to read. Tracing working code between uncommented blocks is not fun. It makes code hard to read and it is almost sure that one day somebody has to waste his or her valuable time to crawl through this horror.

   

• Somebody may get wrong directions from commented code. It may easily happen to somebody new who reads the code to understand how this part of system is working. “Dirty code” like this may contain references to erroneous solution but there is no comment telling us why this code is like it is and how safe it is to use.

    

• Somebody may want to update and use commented code. Newcomers may also want to use that code if they have idea how to modify it. But this code may have deeper problems than they know. It is better to avoid confusing elements in code.

   

• How do I know why this code is here? We can see no comments or explanations about why this code is here. If it is workaround to some problem and this code is there so everybody can use it in the case of emergency then this code needs comments. It must be clear that this code can be used and it is safe to use.

Now there is one more problem. The last method in our example class is not used. But it is there and it has very sweet name. It like says to us that this method must be used somewhere. If somebody starts debugging this class then he or she may try to use this method to get correct results. So it is better to remove this method if it is not used.

As a last thing let’s look at our class when we have removed all the waste.

public class Invoice 
{ 
    private IList<InvoiceLine> _lines; 
 
    // ... 
 
    public decimal Sum() 
    { 
        decimal sum = 0; 
 
        foreach(InvoiceLine line in _lines) 
        { 
            sum += line.CalculateTotal(); 
        } 
 
        return sum; 
    } 
}

The code looks much better now, it is easier to read and it doesn’t contain any informational noise. If you ask me where to keep maybe-in-the-future-needed pieces of code then my answer is simple – put up wiki for your developers and let them keep information there.