HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Examining programming laboratory exercise for faculty

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
laboratoryfacultyexaminingprogrammingexercisefor

Problem

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

Solution

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 to your class declaration.

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

Context

StackExchange Code Review Q#63338, answer score: 4

Revisions (0)

No revisions yet.