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

ISBN validation with Hibernate Validator

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

Problem

I've written a ISBN-validating annotation. It is to be used in conjunction with Hibernate Validator.

I'm pretty satisfied with this code as it works as expected, but I've had to make a design choice that I totally dislike. This is why I post this question.

The method checkISBN returns an Optional regarding the error returned. An empty Optional then means it's okay. Originally, I sincerely thought about using a custom exceptions, but exceptions shouldn't be used as code structure (Effective Java, 2nd edition, item 59). But the Optional used as a negative check totally bugs me. Plus, I feel like the method names are totally wrong, since I'm using Optional in an inverted way and the methods then don't say what they return.

Another point is that I usually favor static methods, but most of my check methods aren't because of method references named ISBNvalidator::check makes it feels like these methods aren't part of this class. Maybe that's just me.

In a lesser measure, please provide some advice on how to properly name my variables that start with ISBN. The classes will stay like that, but I just can't properly decide what to do regarding the variable names: use ISBNAnnotation or isbnAnnotation, for instance. Also, there's some inconsistency in what I decided: String isbn is named like that only because I didn't want to name a variable exactly the same as the annotation ISBN.

Other remarks are naturally welcome, but the previous points are more important in my eyes.

ISBN.java (The annotation created)

```
package myapp.book;

import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import jav

Solution

Why did you include Optional when it doesn't seem to be doing anything special?

To me, it seems you've just used Optional as a replacement for null checks. Which is what it is intended for, but it doesn't give you any real benefits.

I'd just do away with the Optional.

As for naming, IsbnValidator and String isbn and @Isbn is how you'd do it. This to prevent word soup like HTTPURLConnection.

Context

StackExchange Code Review Q#133698, answer score: 3

Revisions (0)

No revisions yet.