Examining programming laboratory exercise for faculty

I’m developing software for examining programming laboratory exercise for faculty. I have an Error object that can be attached to students work. It can be attached to class implementation (mark code in .java file as error), to class itself (student didn’t implement something required) or to the whole solution (something like bad practice error).

public enum RemarkType {
    ERROR, WARNING
}

public enum RemarkBinding {
    FILE_SPECIFIC, TASK_SPECIFIC, ASSIGNMENT_SPECIFIC
}

public class RemarksFactory {       
    public static ErrorRemark createErrorRemark(StudentAssignment studentAssignment,
            StudentTask task, TaskFile taskFile, ExaminerError error,
            String markedCode, int percentage, int startRow, int endRow,
            String explanation) {
        return new ErrorRemark(studentAssignment, task, taskFile, error, markedCode,
                percentage, startRow, endRow, explanation);
    }

    public static WarningRemark createWarningRemark(StudentAssignment student,
            StudentTask task, TaskFile taskFile, String markedCode,
            int startRow, int endRow, String explanation) {
        return new WarningRemark(student, task, taskFile, markedCode,
                startRow, endRow, explanation);
    }
}

public class ErrorRemark {        
    private ExaminerError error;
    private int errorValue;
    private StudentAssignment studentAssignment;
    private StudentTask studentTask;
    private TaskFile taskFile;
    private String explanation;
    private int startRow;
    private int endRow;
    private String markedCode;

    ErrorRemark(StudentAssignment student, StudentTask task, TaskFile taskFile,
                ExaminerError error, String markedCode, int percentage, int startRow,
                int endRow, String explanation) {
            if (startRow > endRow) {
                log.error("startRow(" + startRow
                        + ") row must be less than or equal to endRow(" + endRow
                        + ")");
                throw new IllegalArgumentException();
            }

            setError(error);
            setStudentAssignment(studentAssignment);
            setStudentTask(task);
            setTaskFile(taskFile);
            setMarkedCode(markedCode);
            setPercentage(percentage);
            setExplanation(explanation);

            if (getBinding() == RemarkBinding.FILE_SPECIFIC) {
                setStartRow(startRow);
                setEndRow(endRow);
            } else {
                setStartRow(DEFAULT_ROW);
                setEndRow(DEFAULT_ROW);
            }
        }

    public ExaminerError getError() {
        return error;
    }

    public void setError(ExaminerError error) {
        if (error == null) {
            log.error("error must not be null");
            throw new NullPointerException();
        }

        this.error = error;
    }

    public int getErrorValue() {
        return errorValue;
    }


    public void setErrorValue(int errorValue) {
        this.errorValue = errorValue;
    }

    public StudentAssignment getStudent() {
        return studentAssignment;
    }


    public void setStudentAssignment(StudentAssignment studentAssignment) {
        if (studentAssignment== null) {
            log.error("student must not be null");
            throw new NullPointerException();
        }

        this.studentAssignment = studentAssignment;
    }

    public Task getTask() {
        return studentTask.getTask();
    }

    public StudentTask getStudentTask() {
        return studentTask;
    }

    public void setStudentTask(StudentTask studentTask) {
        this.studentTask = studentTask;
    }

    public TaskFile getTaskFile() {
        return taskFile;
    }

    public void setTaskFile(TaskFile taskFile) {
        this.taskFile = taskFile;
    }

    public String getName() {
        return error.getName();
    }

    public String getDescription() {
        return error.getDescription();
    }

    public String getExplanation() {
        return explanation;
    }

    public void setExplanation(String explanation) {
        this.explanation = explanation == null || "".equals(explanation) ? Remark.DEFAULT_EXPLANATION
                : explanation;
    }

    public RemarkBinding getBinding() {
        if (studentTask == null && taskFile == null) {
            return RemarkBinding .ASSIGNMENT_SPECIFIC;
        } else if (taskFile == null) {
            return RemarkBinding .TASK_SPECIFIC;
        } else {
            return RemarkBinding .FILE_SPECIFIC;
        }
    }

    public String getMarkedCode() {
        return markedCode;
    }

    public void setMarkedCode(String markedCode) {
        this.markedCode = markedCode == null || "".equals(markedCode) ? Remark.DEFAULT_MARKED_CODE
                : markedCode;
    }

    public int getStartRow() {
        return startRow;
    }

    public void setStartRow(int startRow) {
        this.startRow = startRow;
    }

    public int getEndRow() {
        return endRow;
    }

    public void setEndRow(int endRow) {
        this.endRow = endRow;
    }

    public RemarkType getType() {
        return RemarkType.ERROR;
    }

    public int compareTo(Remark o) {
        if (o == null)
            return 1;

        if (getStartRow() < o.getStartRow()) {
            return -1;
        }
        if (getStartRow() > o.getStartRow()) {
            return 1;
        }
        if (getEndRow() < o.getEndRow()) {
            return -1;
        }
        if (getEndRow() > o.getEndRow()) {
            return 1;
        }

        return 0;
    }

    public String toString() {
        String result = "";
        result += "Error:rn" + error + "rn";
        result += "Marked code:rn" + markedCode + "rnrn";
        result += "Explanation: rnt" + explanation + "rn";
        return result;
    }   
}

The only difference between ErrorRemark and WarningRemark is that warning’s error attribute is null (warning has no error).

As you can see, it has a lot of parameters. If error is not attached to file (code in .java file), then attributes like start row, end row and marked code are unused.

The biggest problem for me is that error can be attached to three different things, which creates a lot of combinations and messy code.

Couple of quick remarks:

---

You could consider using your RemarkType enum to use alongside a general Remark class. In the end, the only difference between a Warning and an Error is exactly that: a different severity level. If you have any case-specific logic then you can add those methods to the enum itself.

This would save you from having pretty much the same classes for ErrorRemark and WarningRemark.

---

You’re doing validation inside the constructor instead of the corresponding setters. If I want to, I can create an object that has a startrow less than the endrow but then manually use the setters afterwards to change it anyway.

---

You’re throwing a NullPointerException. Big no-no. What you want to throw is an IllegalArgumentException, much more intuitive. When we catch that exception, we want to catch all invalid arguments; not just those that happen to be null. And it would cause confusion if we receive NPE’s from somewhere else.

---

You have an int errorValue. This doesn’t quite tell me anything, consider using an enum to clarify what each value means in human language (if applicable ofcourse, maybe it is domain-specific).

---

Are you sure those setters should all be public? I would expect a Remark object to be definitive.

---

You implement the compareTo(Remark o) method but you are not implementing the interface, essentially rendering it useless. Add implements Comparable<Remark> to your class declaration.

---

Use platform independent newline constants instead of manually adding rn: System.getProperty("line.separator");