Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Java: a variable "might have already been initialized", but I don't understand how

Tags:

java

final

First some context: all the code pasted below is within another class declared as public class TheClass extends SomeProprietaryClass. I cannot declare these classes in another file for various reasons... And log messages are in French. And I'm a "final happy" kind of programmer. Which is at the core of the problem here...

Now, the code... (probably too much of it -- stripping on demand to only keep the relevant parts)

A custom Exception:

private static final class BreadCrumbException
    extends Exception
{
    private BreadCrumbException(final String message)
    {
        super(message);
    }

    private BreadCrumbException(final String message, final Throwable cause)
    {
        super(message, cause);
    }
}

An enum for "materializing" the visibility of a breadcrumb element:

private enum Visibility
{
    MAINPAGE("R"),
    MENU("M"),
    BREADCRUMB("A"),
    COMMERCIAL("C");

    private static final Map<String, Visibility> reverseMap
        = new HashMap<String, Visibility>();

    private static final String characterClass;

    static {
        final StringBuilder sb = new StringBuilder("[");

        for (final Visibility v: values()) {
            reverseMap.put(v.flag, v);
            sb.append(v.flag);
        }

        sb.append("]");
        characterClass = sb.toString();
    }

    private final String flag;

    Visibility(final String flag)
    {
        this.flag = flag;
    }

    static EnumSet<Visibility> fromBC(final String element)
    {
        final EnumSet<Visibility> result = EnumSet.noneOf(Visibility.class);

        for (final String s: reverseMap.keySet())
            if (element.contains(s))
                result.add(reverseMap.get(s));

        return result;
    }


    static String asCharacterClass()
    {
        return characterClass;
    }

    static String asString(final EnumSet<Visibility> set)
    {
        final StringBuilder sb = new StringBuilder();

        for (final Visibility v: set)
            sb.append(v.flag);

        return sb.toString();
    }

    @Override
    public String toString()
    {
        return flag;
    }
}

A breadcrumb element:

private static class BreadCrumbElement
{
    private static final Pattern p
        = Pattern.compile(String.format("(%s+)(\\d+)",
        Visibility.asCharacterClass()));

    private final String element;
    private final String menuID;
    private final EnumSet<Visibility> visibility;

    BreadCrumbElement(final String element)
    {
        final Matcher m = p.matcher(element);

        if (!m.matches())
            throw new IllegalArgumentException("Élément de fil d'ariane invalide: " + element);

        this.element = element;
        visibility = EnumSet.copyOf(Visibility.fromBC(m.group(1)));
        menuID = m.group(2);
    }

    public boolean visibleFrom(final Visibility v)
    {
        return visibility.contains(v);
    }

    @Override
    public boolean equals(final Object o)
    {
        if (this == o)
            return true;
        if (o == null || getClass() != o.getClass())
            return false;

        final BreadCrumbElement that = (BreadCrumbElement) o;

        return element.equals(that.element);
    }

    @Override
    public int hashCode()
    {
        return element.hashCode();
    }

    @Override
    public String toString()
    {
        return element;
    }

    public String getMenuID()
    {
        return menuID;
    }
}

A breadcrumb:

private static class BreadCrumb
    implements Iterable<BreadCrumbElement>
{
    private static final BreadCrumb EMPTY = new BreadCrumb();

    private final List<BreadCrumbElement> elements
        = new LinkedList<BreadCrumbElement>();

    private String bc;

    BreadCrumb(final String bc)
        throws BreadCrumbException
    {
        final Set<BreadCrumbElement> set = new HashSet<BreadCrumbElement>();
        BreadCrumbElement e;

        for (final String element: bc.split("\\s+")) {
            e = new BreadCrumbElement(element);
            if (!set.add(e))
                throw new BreadCrumbException("Élément dupliqué "
                    + "dans le fil d'Ariane : " +  element);
            elements.add(e);
        }

        if (elements.isEmpty())
            throw new BreadCrumbException("Fil d'ariane vide!");

        if (!elements.get(0).visibleFrom(Visibility.MAINPAGE))
            throw new BreadCrumbException("Le fil d'Ariane ne "
                + "commence pas à l'accueil : " + bc);

        set.clear();
        this.bc = bc;
    }

    private BreadCrumb()
    {
    }

    BreadCrumb reverse()
    {
        final BreadCrumb ret = new BreadCrumb();
        ret.elements.addAll(elements);
        Collections.reverse(ret.elements);
        ret.bc = StringUtils.join(ret.elements, " ");
        return ret;
    }

    public Iterator<BreadCrumbElement> iterator()
    {
        return elements.iterator();
    }

    @Override
    public String toString()
    {
        return bc;
    }
}

The interface to a breadcrumb renderer:

public interface BreadCrumbRender
{
    List<CTObjectBean> getBreadCrumb()
        throws Throwable;

    String getTopCategory();

    String getMenuRoot();

    String getContext();
}

The implementation of the interface above which is the source of my problems:

private class CategoryBreadCrumbRender
    implements BreadCrumbRender
{
    private final BreadCrumb bc;
    private final CTObject object;

    CategoryBreadCrumbRender(final CTObject object)
    {
        this.object = object;
        final String property;

        // FIELD_BC is declared as a private static final String earlier on.
        // logger is also a private static final Logger
        try {
            property = object.getProperty(FIELD_BC);
        } catch (Throwable throwable) {
            logger.fatal("Impossible d'obtenir le champ " + FIELD_BC
                + " de l'objet", throwable);
            bc = BreadCrumb.EMPTY;
            return;
        }

        try {
            bc = new BreadCrumb(property);
        } catch (BreadCrumbException e) {
            logger.fatal("Impossible d'obtenir le fil d'Ariane", e);
            bc = BreadCrumb.EMPTY; // <-- HERE
        }
    }
    // ....

At the point marked // <-- HERE above, Intellij IDEA, which I use, and javac (1.6.0.29) both tell me that Variable bc might already have been assigned to, which is considered an error (and indeed, the code does not compile).

Trouble is, I do not understand why... My reasoning is the following:

  • in the first try/catch block (and yes, .getProperty() does throw Throwable), when an exception is caught, bc gets assigned to successfully, and then I return, so far so good;
  • in the second try/catch block, the constructor may fail, in which case I assign an empty breadcrumb, so it should be OK, even though bc is final: the assignment doesn't happen (?) in the try block but happens in the catch block instead...

Except no, it doesn't. As both IDEA and javac disagree with me, they are certainly right. But why?

(and also, BreadCrumb.EMPTY is declared private static final in the class, I wonder how come I can access it at all... Subsidiary question)

EDIT: there is a known bug with the final keyword (here, thanks to @MiladNaseri for linking to it), however it should be noted that in this bug, variable v is only ever assigned in catch blocks -- but in the code above, I assign it in try blocks and only assign it in catch blocks if an exception is thrown. Also, it should be noted that the error only occurs in the second catch block.

like image 539
fge Avatar asked Jan 15 '12 01:01

fge


2 Answers

Okay, suppose that in the first try block, when doing property = object.getProperty(FIELD_BC); an exception occurs. So, JVM will enter the catch block, and initialize bc along the way.

Then in the second try block, also an exception occurs, resulting in BreadCrumb.EMPTY being assigned to bc, effectively overriding its original value.

Now, that is how bc might have already been initialized. I hope you see where I'm coming from.

Since the JAVAC analysis engine does not draw a distinction between one or many statements inside the try block, it does not see your case any different than the below:

try {
    bc = null;
    String x = null;
    System.out.println(x.toString());
} catch (Throwable e) {
    bc = null;
}

In which case, bc will be assigned twice. In other words, JAVAC won't care that where the source of the Throwable lies, it only cares that it can be there, and that bc might undergo a successful assignment in that try block.

like image 185
Milad Naseri Avatar answered Nov 02 '22 20:11

Milad Naseri


I don't think the analysis is deep enough to really understand that there is only one statement in the try block, and the diagnostic is issued no matter what, so that's why you're seeing it in your case.

like image 1
Bill Avatar answered Nov 02 '22 20:11

Bill