Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

To provide some background context, I'm implementing a web-based solution (Java, Spring, Hibernate) that allows the creation/authoring of task-centric workflow documents. Basically a workflow provides an ordered collection of tasks (and related content) for a user to step through.

One of the available task types is a "risk assessment" matrix, in which the user selects a "risk" level (i.e. "how likely is it that this thing will break?") and a "consequence" level (i.e. "how bad is it if the thing does break?"). Based upon their input, some text provided by the workflow author will be displayed.

Essentially these tasks are backed by a 5x5 matrix of strings, and by selecting a "risk" and "consequence" level, the user is indicating a position in the matrix.

There's one further complication, in that all text provided by the document author must support internationalization. To deal with this I've implemented an InternationalText entity, which simply refers to a collection of LocalText entities, each one of which specifies a language, and a string of text in that language. That all works fine.

What I'm wondering about is, what's the best way to store the matrix itself in the data model? I can't just serialize an array of Strings, as each position in the matrix is actually occupied by an InternationalText instance and not a String. I suppose maybe I could serialize an array of InternationalText ids, but that seems fairly hacky, doesn't it?

Under my current approach, I've explicitly declared a field dedicated to each position in the matrix. That makes for 25 getters and 25 setters. To avoid having to deal with that nonsense, I added some helper methods that take advantage of reflection in order to get and set fields based upon their matrix position, in standard (x, y) notation (I use a two-dimensional JSONArray to transport the matrix data to/from the rest of the application).

What I'm really after is the cleanest, most convenient way of managing the matrix content. I think what I have is fairly reasonable, but can anyone think of a better approach?

Here's the code:

@Entity
@Table(name = "matrixText")
public class MatrixText extends AccountItem {
    private static final Logger LOG = Logger.getLogger(MatrixText.class);

    //fields

    //relationships
    private TemplateQuestion template;

    //XXX:  addressing is <consequence>_<risk> == <x>_<y>
    private InternationalText text0_0;
    private InternationalText text0_1;
    private InternationalText text0_2;
    private InternationalText text0_3;
    private InternationalText text0_4;

    private InternationalText text1_0;
    private InternationalText text1_1;
    private InternationalText text1_2;
    private InternationalText text1_3;
    private InternationalText text1_4;

    private InternationalText text2_0;
    private InternationalText text2_1;
    private InternationalText text2_2;
    private InternationalText text2_3;
    private InternationalText text2_4;

    private InternationalText text3_0;
    private InternationalText text3_1;
    private InternationalText text3_2;
    private InternationalText text3_3;
    private InternationalText text3_4;

    private InternationalText text4_0;
    private InternationalText text4_1;
    private InternationalText text4_2;
    private InternationalText text4_3;
    private InternationalText text4_4;

    public MatrixText() {
        //all fields default to null
    }

    @OneToOne(optional = false, fetch=FetchType.LAZY)
    public TemplateQuestion getTemplate() {
        return template;
    }
    public void setTemplate(TemplateQuestion template) {
        this.template = template;
    }

    @ManyToOne(optional = true, fetch=FetchType.LAZY)
    public InternationalText getText0_0() {
        return text0_0;
    }
    public void setText0_0(InternationalText text0_0) {
        this.text0_0 = text0_0;
    }

    //...

    @ManyToOne(optional = true, fetch=FetchType.LAZY)
    public InternationalText getText4_4() {
        return text4_4;
    }
    public void setText4_4(InternationalText text4_4) {
        this.text4_4 = text4_4;
    }

    @Transient
    public InternationalText getTextAtCoordinate(int x, int y) {
        //XXX:  x=<consequence>, y=<risk>
        if (x < 0 || x > 4 || y < 0 || y > 4) {
            //invalid coordinate
            return null;
        }

        String methodName = "getText" + x + "_" + y;
        try {
            return (InternationalText)this.getClass().getMethod(methodName).invoke(this);
        }
        catch (Exception e) {
            LOG.error("Unable to find/invoke getter with name=" + methodName);
        }

        //couldn't invoke the getter
        return null;
    }

    @Transient
    public void setTextAtCoordinate(int x, int y, InternationalText text) {
        if (x < 0 || x > 4 || y < 0 || y > 4) {
            //invalid coordinate
            return;
        }

        String methodName = "setText" + x + "_" + y;
        try {
            this.getClass().getMethod(methodName, InternationalText.class).invoke(this, text);
        }
        catch (Exception e) {
            LOG.error("Unable to find/invoke setter with name=" + methodName);
        }
    }

    @SuppressWarnings("unchecked")
    @Override
    @Transient
    public JSONObject getJson() {
        JSONObject result = new JSONObject();
        JSONArray matrix = new JSONArray();

        for (int x = 0; x < 5; x++) {
            JSONArray col = new JSONArray();
            matrix.add(col);
            for (int y = 0; y < 5; y++) {
                InternationalText text = this.getTextAtCoordinate(x, y);
                col.add(text == null ? null : text.getId());
            }
        }

        //we need to put the matrix inside of a JSONObject due to constraints imposed by inheritance
        result.put("matrix", matrix);

        return result;
    }

    @SuppressWarnings("unchecked")
    @Transient
    public JSONObject getJson(Language language) {
        JSONObject result = new JSONObject();
        JSONArray matrix = new JSONArray();

        for (int x = 0; x < 5; x++) {
            JSONArray col = new JSONArray();
            matrix.add(col);
            for (int y = 0; y < 5; y++) {
                InternationalText text = this.getTextAtCoordinate(x, y);
                col.add(text == null ? null : text.getTextForLanguage(language));
            }
        }

        //we need to put the matrix inside of a JSONObject due to constraints imposed by inheritance
        result.put("matrix", matrix);

        return result;
    }

If you'd like something a bit less abstract, here's a screenshot of what it looks like in the UI:

enter image description here

share|improve this question
2  
Why can't you have a List of InternationalTexts? –  abuzittin gillifirca Feb 14 '13 at 12:47
    
That may be a possibility, combined with the @OrderColumn annotation. However I'm not sure if that will play well with null elements in the collection (as you can see from my implementation, the text for any given matrix position is allowed to be null). Do you know if that's supported? Also it would require some custom code to enforce that the list always has the correct number of things in it when persisting the entity. –  aroth Feb 14 '13 at 13:01

1 Answer 1

  1. I'd extract out a validation method:

    private boolean isValidCoordinates(final int x, final int y) {
        if (x < 0 || x > 4 || y < 0 || y > 4) {
            return false;
        }
        return true;
    }
    

    Currently it's duplicated.

  2. 0, 4, 5 should be named constants. It would improve readability.

  3. Instead of only logging and swallowing the exceptions throw an IllegalStateException. It seems to me that if it happens you lost some data. Consider crashing early. See: The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.

  4. I'd try creating a class for the <consequence>_<risk> pair and use it as a key in a map. Values could be InternationalText instances:

    // it should have a better name
    public class ConsequenceLevelRiskLevelPair {
        private int consequenceLevelRiskLevelPairId;
    
        private ConsequenceLevel consequenceLevel;
        private RiskLevel riskLevel;
    
        // TODO: hashCode and equals here
    }
    
    
    public class MatrixText extends AccountItem {
    
        ...
    
        private Map<ConsequenceLevelRiskLevelPair, InternationalText> texts = ...
    
    
    }
    

    ConsequenceLevel would contain the name of the level and an ID. RiskLevel is similar with risk level name.

    I have not used Hibernate/JPA recently (and have not tried the solution above) but I guess Hibernate can handle maps with non-string keys. This Stack Overflow question also could help: How to map a 2-d matrix in Java to Hibernate/JPA?

    It would make the reflection unnecessary.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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