Simple interface and class for holding text to be analyzed

Question

I am making a sentiment analysis library targeted first at social media text (like Twitter) and then I will expand it for more general use.

For the first step I am making a simple interface and a class that is going to be used for storing some data about the text to be analyzed like the text itself, it's manually annotated sentiment and the calculated sentiment that must be set only once (that's my current design anyway).

ObjectForAnalysis.java interface:

package com.github.dkanellis.sa_for_socialmedia.sentiment.objects;

public interface ObjectForAnalysis {

    double getManuallyAnnotatedMeanSentiment();

    double getCalculatedMeanSentiment();

    boolean isCalculated();

    void setCalculatedMeanSentiment(double calculatedMeanSentiment);
}

TwitterStatus.java implementation:

package com.github.dkanellis.sa_for_socialmedia.sentiment.objects;

public class TwitterStatus implements ObjectForAnalysis {

    private final long id;
    private final String text;
    private final double manuallyAnnotatedMeanSentiment;
    private double calculatedMeanSentiment;
    private boolean calculated;

    public TwitterStatus(long id, String text, double manuallyAnnotatedMeanSentiment) {
        this.id = id;
        this.text = text;
        this.manuallyAnnotatedMeanSentiment = manuallyAnnotatedMeanSentiment;
        this.calculated = false;
    }

    @Override
    public double getManuallyAnnotatedMeanSentiment() {
        return manuallyAnnotatedMeanSentiment;
    }

    @Override
    public double getCalculatedMeanSentiment() {
        if (isCalculated()) {
            return calculatedMeanSentiment;
        } else {
            throw new IllegalStateException("Mean sentiment has not been calculated yet.");
        }
    }

    @Override
    public boolean isCalculated() {
        return calculated;
    }

    @Override
    public void setCalculatedMeanSentiment(double calculatedMeanSentiment) {
        if (isCalculated()) {
            throw new IllegalStateException("Mean sentiment has already been calculated.");
        } else {
            this.calculatedMeanSentiment = calculatedMeanSentiment;
            calculated = true;
        }
    }

    @Override
    public String toString() {
        StringBuilder buffer = new StringBuilder();
        buffer.append("Tweet: ").append(id).append("\n");
        buffer.append("Text: ").append(text).append("\n");
        buffer.append("Manual Annotation: ").append(manuallyAnnotatedMeanSentiment).append("\n");
        buffer.append("Calculated Annotation: ")
                .append(isCalculated() ? "Not calculated" : calculatedMeanSentiment).append("\n");

        return buffer.toString();
    }

}

Feel free to critique anything from package naming to class naming to the way I implemented my constraint of having the mean sentiment set only once.

Answer 1

Your concrete TwitterStatus class would, in a single-threaded application, be OK for the purpose of having a single-set ("lazy set", or "set and freeze") mechanism. Two concurrent threads each setting the value, or one thread setting, and another thread getting the value may cause problems though.

The situation can be relatively easily resolved with an AtomicReference value though...

Consider the following value that replaces both the calculatedMeanSentiment and calculated fields:

private final AtomicReference<Double> calculatedMeanSentiment = new AtomicReference<>(null);

Then, in your methods, you can have:

@Override
public double getCalculatedMeanSentiment() {
    Double val = calculatedMeanSentiment.get();
    if (val == null) {
        throw new IllegalStateException("Mean sentiment has not been calculated yet.");
    }
    return val;
}

@Override
public void setCalculatedMeanSentiment(double calculatedMeanSentiment) {
    if (!this.calculatedMeanSentiment.compareAndSet(null, calculatedMeanSentiment)) {
        throw new IllegalStateException("Mean sentiment has already been calculated.");
    }
}

@Override
public boolean isCalculated() {
    return calculatedMeanSentiment.get() != null;
}

Notice also that when you throw, or return from the 'if' part of an if statement, there is no need to have the 'else' part. Specifically, you have structures like this:

    if (isCalculated()) {
        return calculatedMeanSentiment;
    } else {
        throw new IllegalStateException("Mean sentiment has not been calculated yet.");
    }

that return in the if. They can be easily rewritten as 'guard conditions':

    if (!isCalculated()) {
        throw new IllegalStateException("Mean sentiment has not been calculated yet.");
    }
    return calculatedMeanSentiment;

Answer 2

The code is good, but there is something that I dislike, this bit

 @Override
public double getCalculatedMeanSentiment() {
    if (isCalculated()) {
        return calculatedMeanSentiment;
    } else {
        throw new IllegalStateException("Mean sentiment has not been calculated yet.");
    }
}

I don't like the idea of using exceptions to handle program's state, it assumes that the user of the class is always aware of them. I would simply use a monadic type like Optional

 @Override
public Optional<Double> getCalculatedMeanSentiment() {
    if (isCalculated()) {
        return Optional.of(calculatedMeanSentiment);
    } else {
       return Optional.absent();
    }
}

This gives a clearer hint to class users that the value they asking for might not be there.

If using Java 8 then Optional is there, otherwise Guava is your friend