4
\$\begingroup\$

In the code below, does not clearing or setting the local variable completed to null create the potential for a memory leak?

private Vector<Element> elements;

private void update()
{
    Vector<Element> completed = new Vector<Element>();
    for( Element e : elements )
    {
        if( e.isComplete() )
        {
           completed.add( e );
        } else {
           e.update();
        }
    } 

    elements.removeAll( complete );
}

Should I add complete.clear() before the update() method exits?

The elements Vector is an instance variable on a class that is populated with Element objects elsewhere in the program.

\$\endgroup\$
3
\$\begingroup\$

Because complete is used and is referenced entirely within the context of the method update(), it will become unreferenced after the method returns.

This will eventually cause it to be garbage collected.

After it has been garbage collected, some or all of the elements that belong to it will also become unreferenced and then garbage collected.

So technically, no, it is not necessary to call complete.clear().

On the other hand, getting into the habit of insuring that things are properly cleaned up can be important in other circumstances. As such, it would not be wrong to add the clear() call to the end of the method. To do so, or not do so could be either a stylistic choice, or something to consider addressing in a coding standards document. I would probably add it myself.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the info. I was pretty sure that this was the case, however Java is not my native language so it seemed wise to get a second opinion. \$\endgroup\$ – JeremyFromEarth Aug 23 '12 at 2:57
  • \$\begingroup\$ Actually it would be more efficient to just let the garbage collector claim the entire list rather than manually clearing it. \$\endgroup\$ – casablanca Aug 23 '12 at 6:24
3
\$\begingroup\$

Two things. First, you really shouldn't be using Vector. See https://stackoverflow.com/questions/1386275/why-is-java-vector-class-considered-obsolete-or-deprecated

Second, I would not clear the list. The garbage collector will handle it for you and unless you have a very specific reason to not rely on the GC to do its job, there is no reason to clear it. Clearing it may not seem to have any downsides, however, you are adding code that is not needed. Although it may seem trivial, having these calls all over a program will only serve to decrease the readability of the code. When the code is revisited later, it may be confusing as to why the collections are being cleared when they are going out of scope anyway.

\$\endgroup\$
  • \$\begingroup\$ Good to know! Looks like ArrayList is the way to go. \$\endgroup\$ – JeremyFromEarth Aug 23 '12 at 15:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.