3

I have two paths:

  • /students
  • /students/{id}/addresses

...with the following behavior:

POST to /students - 201 Created (if successfully created the Student) POST to /students/{id}/addresses - 201 Created (if successfully created the Address under the requested user) Now, imagine a situation where the client POST to /students/12/addresses, but there's no Student with id = 12.

How should I handle this scenario?

Should I insert without any check and wait for DAO to throw the Exceptions or, before inserting, I should check for Student's existance?

1st option:

StudentController.java

@Autowired private AddressService addressService; @PostMapping("/students/{id}/addresses") public ResponseEntity<?> handleRequestOfCreateAddressToStudent(@PathVariable("id") Long studentId, @RequestBody Address address) { address.setStudent(new Student(studentId)); Address createdAddress = addressService.saveAddress(address); URI location = ServletUriComponentsBuilder.fromCurrentRequest().path("/{id}").build(createdAddress.getId()); return ResponseEntity.created(location).build(); }

AddressService.java

@Autowired private AddressRepository addressRepository; public Address saveAddress(Address address) { // omitted for brevity... return addressRepository.save(address); }

This way, since I'm using Spring framework, the line return addressRepository.save(address); would throw a DataIntegrityViolationException, because the constraint is violated (there's no Student with such ID to bind with that Address). And, since I need to send the response to the client, I would have to translate the exception that is throwed.

2nd option:

StudentController.java

@Autowired private StudentService studentService; @Autowired private AddressService addressService; @PostMapping("/students/{id}/addresses") public ResponseEntity<?> handleRequestOfCreateAddressToStudent(@PathVariable("id") Long studentId, @RequestBody Address address) throws StudentNotFoundException { Student student = studentService.findById(studentId); if(student == null) throw new StudentNotFoundException("message"); address.setStudent(new Student(studentId)); Address createdAddress = addressService.saveAddress(address); URI location = ServletUriComponentsBuilder.fromCurrentRequest().path("/{id}").build(createdAddress.getId()); return ResponseEntity.created(location).build(); }

This way, I don't need to translate the Exception, as I already know that the Student does not exist so that I can throw a custom exception.

My thoughts in both approaches:

  1. Without checking, I do reduce the code and the number of times I trigger my database;
  2. Checking, I do know exactly why I'm unable to add the Address (the Student does not exist) and can throw a custom exception without having to translate it;

Which one is the best approach to this scenario?

Should I wait for the DAO to throw the Exception, or should I prevent it from throwing by checking if the Student exists?

Improve this question
3
  • Which way provides the most benefits for the least cost? Which way most effectively meets your specific requirements? Commented Dec 17, 2018 at 16:20
  • Why is someone adding an address for a non-existing user in the first place? Commented Dec 18, 2018 at 5:39
  • Well, that's an exception. I want to have mapped and handled as much scenarios as I can. Commented Dec 18, 2018 at 14:01

4 Answers 4

Reset to default
1

You should never use exceptions as a flow-control device. Check for the existence of the Student first, then add the address. As you noted, this give you more control over the messaging you send, plus you keep all the logic in the same flow (as opposed to returning messages from both the main block and the exception handler).

Improve this answer
4
  • Well, that's a great point. Not use exceptions as a flow-control. Thank you! Commented Dec 17, 2018 at 17:01
  • @MatheusCirillo Using exceptions for control-flow might have significant cost or not, depending on language and implementation. Still, whether you use exceptions for handling errors doesn't seem the most important point. Commented Dec 18, 2018 at 17:14
  • @Deduplicator: Using exceptions for control flow makes it very difficult to reason about the behavior of the code, and makes it almost impossible for optimizers to do their job -- not just on the code in question, but any code that invokes it. Commented Jan 7, 2019 at 16:17
  • @TMN For arbitrary control-flow, and unexceptional situations, yes, exceptions are generally the wrong tool. Still, there are languages and implementations where exceptions are business-as-usual, instead of an exception. Commented Jan 7, 2019 at 16:43
4

It may certainly be a valid concern to reduce latency and demand on the backend for performance reasons. But that pales in comparison to correctness:

TOCTOU, fully written as Time-Of-Check-Time-Of-Use, is a very well known error coming from thinking in a strictly linear model (like cooperative multi-threading in Windows 3.11 was) while you actually have independant, concurrent, non-synchronized actions.

Well, unless you also support deleting of students, at least if you found the student, it will still be there later. Can you guarantee that? Also, that it won't ever change? Just go for the more performant and always correct approach.

Some say exceptions are a performance-hog, and depending on language and implementation they might not be wrong. Measure it, and if that's the case, error-codes are also an option. There is a reason many libraries feature various TryXYZ() functions in addition to ParseXYZ(), as failure might be common.

Improve this answer
0

That business exception should be thrown by your service layer. Your persistence layer should throw a data exception. Then, in your controller, you'll only translate that business exception to an HTTP response.

This is an exceptional case (who's requesting to your REST API about a non existing student?) so the handling should be as such.

Improve this answer
0

If we want to be strict towards HTTP I will return an HTTP 404 Not found.

Since the URL doesn't point to an existing location you can't find it, thus 404.

For your service layer if you want a detailled exception handling, then you should have a StudentNotFoundException (or eventually a more generic EntityNotFoundException) which you would translate to 404. For the implementation, whether you try to search for student before inserting of try to insert straight into the table doesn't matter a lot. Unless you're going to have hundred of thousands tries of inserts. If you need ever need a classic "import initialization data" you will likely need a proper code just to handle that specific case, no need to have your main service worrying about that one case.

Note that here using Exception is not flow control, it is really an exception that lead to answer to a standard error.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.