Avoid Redundant Calls

It's important as developers that we are careful to not make unnecessary, redundant method calls. During code reviews, I often come across situations where code is doing this extra, unneeded work. In the short term, it may make the code slightly slower. But longer term, as code is updated for future versions, redundant calls could have more drastic effects on performance.

In a simple situation, the same getLocalData() method is called twice, for no particular reason:

Data fetchData()
{
    if (getLocalData() != null)
        return getLocalData().getData();
    return null;
}

This should have been written as:

Data fetchData()
{
    ComplexData data = getLocalData();
    if (data != null)
        return data.getData();
    return null;
}

Sometimes programmers think this is not much of a problem, because perhaps they know that getLocalData() is pretty simple:

ComplexData getLocalData()
{
    return data;
}

But we run into problems with code maintainability and update ability. Let's say for the next version of the code, when the functional requirements change, and getLocalData() now has to access an external data repository, or something much slower than simple memory access. That original double call to getLocalData() can have a much bigger effect.

That effect is magnified when the method is called in a loop. Take this code for example:

void processData()
{
    for (Item item : list)
    {
        process(getLocalData(), item)
    }
}

Developers often test against smaller data sets than found in production environments. I've seen situations were a developer was repeatedly calling a method that accessed a database. In this local testing, there was no performance problem, because the data set was small. But in production, on a much larger data set, his new code made the application unusable.

I recently came across code similar to this:

boolean hasAny()
{
    if ((findTheData() != null) && (findTheData().size() > 0))
        return true;
    return false;
}

In this situation, findTheData() was a complex method, doing lots of work, but the developer didn't pick up on the issue of the double, redundant call. This should be a red flag to any reader of this code.

This also highlights where good helper methods come into play. Developers often write code in the pattern: (list != null) && (list.size() > 0). A simple isEmpty() helper benefits this code:

static boolean isEmpty(Collection collection)
{
    return (collection == null) || (collection.size() == 0);
}

This helper removes the double call to findTheData(), by forcing the result into a parameter variable. The hasAny() method can become:

boolean hasAny()
{
    return !isEmpty(findTheData());
}

Conclusion

It is important for developers to really consider the method calls in their code and think about the effects of those calls.