5 ms·
I think it all depends on context. I use checked exceptions for handling edge cases in Java, which turns something like this: public Session startByIntervi
by mtrimpe 14y ago
I think it all depends on context. I use checked exceptions for handling edge cases in Java, which turns something like this:
public Session startByInterviewId(Long interviewId, String email, String name) {
Session session;
Interview interview = interviewService.getPublicById(interviewId);
if (interview != null) {
Account user = userService.createUser(name, email, interview.getLocale());
if (user != null) {
session = startByInterview(interview, user);
}
}
return session;
}
into this:
public Session startByInterviewId(Long interviewId, String email, String name)
throws Interview.NotActive, Interview.NotPublic, Interview.DoesNotExist, Account.EmailExists {
Interview interview = interviewService.getPublicById(interviewId);
Account user = userService.createUser(name, email, interview.getLocale());
return startByInterview(interview, user);
}
in which case I'm very much in favor of using exceptions as control flow mechanisms.
Instead of just getting nulls to indicate failure I now even know exactly what caused my error.
I would never even think of writing code like this in JavaScript or Clojure though and I'd guess is that in Scala/Haskell a custom Option would be much better.
- arethuza 14y agoInteresting, I first looked at the second code segment and really didn't like it. However, I think thats just the whole Java checked exception thing (which I think everyone now regards as a mistake) - if you removed the throws and everything after it on that line it actually looks quite sensible and much better than simply returning null to mean "it didn't work for some reason I'm not going to tell you about".
- mtrimpe 14y agoExactly. It's essentially as close as you can (legibly) get to an Option/Maybe monad in Java.
- mullr 14y agoThe Option/Maybe monad seems the way to do it. The null object pattern could also apply here, but you have to be pretty disciplined across your project to make it really work. Seems like the kind of thing you need language support for. Actually: while the second code block is more readable, if I encountered it in the wild I would assume that it's full of bugs. It just doesn't seem very idiomatic. I'd much rather take advantage of the 'goto' nature of the return statement: public Session startByInterviewId(Long interviewId, String email, String name) { Interview interview = interviewService.getPublicById(interviewId); if(interview == null) { return null; } Account user = userService.createUser(name, email, interview.getLocale()); if(user == null) { return null; } return startByInterview(interview, user); } There we go. 5 lines, one level of indentation for the main path, and semantics which are obvious to anybody who reads it. But your point about the exceptions adding more information is well taken.
- deleted 14y ago[deleted]
- bruceboughton 14y agoFixed your original code for you: public Session startByInterviewId(Long interviewId, String email, String name) { Interview interview = interviewService.getPublicById(interviewId); if (interview == null) { return null; } Account user = userService.createUser(name, email, interview.getLocale()); if (user == null) { return null; } return startByInterview(interview, user); } Besides that, you have not magically reduced LOCs, you have moved them elsewhere so these code blocks are not equivalent.
- mtrimpe 14y agoWe could always say that you shouldn't use return statements for control flow to complicate things further. ;) Your code doesn't preserve information about what caused the failure though. And also, I actually did magically reduce lines of code since I can just let the exceptions I fix bubble up and I've customized my web framework to turn them into appropriate error pages and the the <Class>.NotFound exceptions are even annotated so that they're automatically turned into 404s.
- bjourne 14y agoOuch. Java's checked exception straightjacket would mean that every code that calls startByInterviewId() would either have to handle exceptions or declare "throws Interview.NotActive, Interview.NotPublic, Interview.DoesNotExist, Account.EmailExists". The longer up in the callchain, the longer the throws-list becomes until the programmer either gives up and writes "throws Exception" or is forced to handle the exceptions in a half-assed way. Likely by writing an empty catch-clause and hoping that code piece is never reached. It's especially unnerving for the user of the method if they are sure the input data is valid. For example, if interviewId comes directly from the database, then Interview.DoesNotExist can't happen so having to catch it shouldn't be needed.
- dcminter 14y agoI would expect those exceptions to implement a Throwable interface, perhaps ServiceException. If you don't care about the specifics of the error then you catch that and handle it. If you do care then you're going to need to handle the specific cases anyway. Personally I like checked exceptions at layer boundaries.