New programmers have an aversion to creating new variables and will try and reuse existing ones. New variables have no real cost other than a few seconds of typing.
In this next example, the author's aversion to creating new variables made the code difficult to comprehend. The variable amount represents more than one kind of amount:
Set invoiceFlag = new HashSet();
for (Invoice invoice: getUnprocessedInvoices())
{
: // 200 lines snipped here
:
invoiceFlag.add((Object)(invoice.getId()));
Amount amount = temp.getRatePerDay();
row.addCell(new AmountCell(amount));
:
amount = temp.getRatePerHour();
row.addCell(new AmountCell(amount));
}
There are a number of problems in this example:
Naming of invoiceFlag. A flag is supposed to be a boolean, but further examination shows it to be a set of invoice numbers already processed so they do not get processed twice.
Naming of the invoice variable. Normally such a generic name is not a problem in a small loop, but this loop was over 200 lines in size and invoice was defined far back at the top of the loop. So it would be better to assign a more descriptive name to that variable, like currentInvoice.
Incorrect usage of a temporary variable. temp is actually used to store “a monetary expense on the invoice” – of enough importance to elevate it above temporary variable status. One clue that temp was misused is that it is referred to more than once and not on subsequent lines.
The amount variable represents different concepts at different locations in the loop. This variable was first used to refer to “rate per day”. Then some time later in the loop, it was used to refer to a totally unrelated variable “rate per hour”. Imagine the consequences if a developer came along later to maintain the code and assumed the amount was set to “rate per day” throughout the loop.
A more sensible solution is below”:
// [Long] invoice ID for already-shown invoices
Set alreadySeenInvoices = new HashSet();
for (Invoice currentInvoice: getUnprocessedInvoices())
{
: // 200 lines snipped here
:
alreadySeenInvoices.add(currentInvoice.getId());
Amount ratePerDay = currentInvoice.getRatePerDay();
row.addCell(new AmountCell(ratePerDay));
:
Amount ratePerHour = currentInvoice.getRatePerHour();
row.addCell(new AmountCell(ratePerHour));
}
