If the name of a method implies it performs a particular function then it is a good idea to ensure that the primary activity matches the name.  It can be confusing to the reader to do the opposite of the implied activity and then negate the result.

The code in the example below is not strictly wrong, but the primary focus of its existence appears to be to see if the user is not linked to the account.

private boolean currentUserOwnsAccount(UserContext context, long accountId)
{
    if (context == null ||
        context.getLinkedAccountId() == null ||
        context.getLinkedAccountId().longValue() != accountId)
    {
        return false;
    }
    return true;
}

In the improved version of the code below, the method performs its “primary function” which is to find out if the current user owns an account, as the focus of all its activity.

private boolean currentUserOwnsAccount(UserContext context, long accountId)
{
    if (context != null &&
        context.getLinkedAccountId() != null &&
        context.getLinkedAccountId().longValue() == accountId)
    {
        return true;
    }
    return false;
}

Exception: This rule only applies where it takes roughly the same amount of code to do one or the other, and complexity is not increased.

blog comments powered by Disqus