Question

Let's assume we have a class Student with the following constructor:

/** Initializes a student instance.
 * @param matrNr    matriculation number (allowed range: 10000 to 99999)
 * @param firstName first name (at least 3 characters, no whitespace) 
 */
public Student(int matrNr, String firstName) {
    if (matrNr < 10000 || matrNr > 99999 || !firstName.matches("[^\\s]{3,}"))
        throw new IllegalArgumentException("Pre-conditions not fulfilled");
    // we're safe at this point.
}

Correct me if I'm wrong, but I think in this example, I followed the design by contract paradigm by simply specifiying the (rather static) constraints on the possible input values and raising a generic, unchecked exception if those are not fulfilled.

Now, there is a backend class that manages a list of students, indexed by their matriculation number. It holds a Map<Integer, Student> to save this mapping and provides access to it through an addStudent method:

public void addStudent(Student student) {
    students.put(student.getMatrNr(), student);
}

Now let's assume there is a constraint on this method like "a student with the same matriculation number must not already exist in the database".

I see two options of how this could be realized:

Option A

Define a custom UniquenessException class that is raise by addStudent if a student with the same matr. number already exists. Calling code will then look something like this:

try {
    campus.addStudent(new Student(...));
catch (UniquenessError) {
    printError("student already existing.");
}

Option B

State the requirement as a pre-condition and simply raise an IAE if it doesn't hold. Additionally, provide a method canAddStudent(Student stud) that checks in advance whether addStudent will fail. Calling code would then look something like this:

Student stud = new Student(...);
if (campus.canAddStudent(stud))
    campus.addStudent(stud);
else
    printError("student already existing.");

I feel that option A is much cleaner from a software-engineering point of view, for at least the following reason:

  • It can easily be made thread-safe without modifying the calling code (Thanks to Voo for pointing me to TOCTTOU, which seems to describe that exact issue)

Thus I wonder:

  1. Is there a third option which is even better?
  2. Does option B have an advantage that I didn't think of?
  3. Would it actually be allowed from a design by contract point of view to use option B and define the uniqueness as a pre-condition of the addStudent method?
  4. Is there a rule of thumb when to define pre-conditions and simply raise IAE and when to use "proper" exceptions? I think "make it a pre-condition unless it depends on the current state of the system" could be such a rule. Is there a better?

UPDATE: It seems like there is another good option, which is to provide a public boolean tryAddStudent(...) method that doesn't throw an exception but instead signals error/failure using the return value.

Was it helpful?

Solution

(this is too long for a comment)

In your option B, I wouldn't use a Map<Integer,Student> and then do:

if (campus.hasStudent(12000)) 
    printError("student already existing.");
else
    campus.addStudent(new Student(...));

The Map abstraction isn't practical enough for your use case (you're mentionning concurrency issues), I'd use instead a ConcurrentMap<Integer,Student> and do something like this:

final Student candidate = new Student(...);
final Student res = putIfAbsent(student.getMatrNr(), candidate)
if ( res != null ) {
    throw new IllegalStateException("Class contract violation: \"student already exists!\", please read the doc");
}

OTHER TIPS

I don't believe the way that the backend class manages a list of students would be relevant to the contract--that is, that it holds a Map<Integer, Student> would not be part of the contract. Thus bringing the matriculation number into the contract in hasStudent(int matrNr) seems a little evil too.

I'd suggest the the campus probably should have a method Boolean hasStudent(Student student), which would check to see if the campus has the student based on whatever the condition. If uniqueness is required by contract, and is truly exceptional, you would then use the contractual check:

   Student student= new Student(int matrNbr, String name);
   if (campus.hasStudent(student) {
      throw new UniquenessException();
   }
   else {
      campus.add(student);
   }

The exceptions thrown should be as relevant to the contract as the arguments and return values

UPDATE

If the add should simply fail if uniqueness is not met and is not exceptional, then don't throw the exception. Instead, make the success of the add a return value (as in java.util.HashSet.add()). This way, campus.add(Student) would return true if the student was actually added.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top