Where a variable is defined tells the programmer a lot about scope and usage, especially when it comes time to refactor a body of code that has grown into a monster.  Define the variable close to where it is going to be used.

In the following example, aboutMe is defined in class scope so it can be accessed by all methods in the class, and that it relates to the instance of ViewProfileController.

public class ViewProfileController extends BaseCommandController
{
    private String aboutMe;
    public static final String PROPERTY_ABOUTME = “aboutMe”;
    public static final int MAX_TITLE_LEN = 50;

    protected Map getReferenceData(long profileId)
    {
        UserProfile profile = serviceFacade.getProfile(profileId);
        aboutMe = profile.getAboutMe();
        if (aboutMe.length() > MAX_TITLE_LEN)
        {
           aboutMe = aboutMe.substring(0,MAX_TITLE_LEN);
        }
        Map<String, Object> map = new HashMap<String, Object>();
        map.put(PROPERTY_ABOUTME, aboutMe);
        return map;
    }
}  

The experienced developer sees 'aboutMe' defined as a class-scope member variable and assumes it will be used in many methods on the ViewProfileController class.

But this raises questions - all Controller instances can be reused by more than one thread at a time, so it does not make sense to have member variables in a controller.  A closer review of the code shows that aboutMe is only used in the getReferenceData() method, and should have been defined inside that method.

Therefore, it should be moved to where it is used:

public class ViewProfileController extends BaseCommandController
{
    public static final String PROPERTY_ABOUTME = “aboutMe”;
    public static final int MAX_TITLE_LEN = 50;

    protected Map getReferenceData(long profileId)
    {
        UserProfile profile = serviceFacade.getProfile(profileId);
        String aboutMe = profile.getAboutMe();
        if (aboutMe.length() > MAX_TITLE_LEN)
        {
           aboutMe = aboutMe.substring(0,MAX_TITLE_LEN);
        }
        Map<String, Object> map = new HashMap<String, Object>();
        map.put(PROPERTY_ABOUTME, aboutMe);
        return map;
    }
}

The aboutMe is created only at the point where it is needed and also happens to fix a multi-threading bug.  There is no practical execution speed advantage in moving the variable out to the object level.

blog comments powered by Disqus