Add Enum and Boolean fields validators #190
Conversation
dmytro-kashcheiev
assigned
dmytro-kashcheiev and
alexander-yevsyukov and unassigned
dmytro-kashcheiev
Current coverage is 78.15% (diff: 96.96%)@@ master #190 diff @@
==========================================
Files 130 132 +2
Lines 4709 4733 +24
Methods 0 0
Messages 0 0
Branches 518 521 +3
==========================================
+ Hits 3678 3699 +21
- Misses 937 939 +2
- Partials 94 95 +1
|
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
Please see my comments. Some minor changes are required.
| * @author Dmitry Kashcheiev | ||
| */ | ||
| /* package */ class BooleanFieldValidator extends FieldValidator<Boolean> { | ||
|
|
There was a problem hiding this comment.
Please remove excessive empty line.
|
|
||
| @Override | ||
| protected boolean isValueNotSet(Boolean value) { | ||
| // Boolean field can't be determined if it set or not, because protobuf 'false' value and 'no value' are the same |
There was a problem hiding this comment.
A Javadoc would be more appropriate here.
| @Override | ||
| protected boolean isValueNotSet(Boolean value) { | ||
| // Boolean field can't be determined if it set or not, because protobuf 'false' value and 'no value' are the same | ||
| return false; |
There was a problem hiding this comment.
Please test this. Do you have a Codecov browser plug-in installed?
|
|
||
| @Override | ||
| protected boolean isValueNotSet(Descriptors.EnumValueDescriptor value) { | ||
| return value.getNumber() == 0; |
There was a problem hiding this comment.
Have final boolean result field immediately before return. This way it's easier to debug, when needed.
There was a problem hiding this comment.
added final boolean result also updated for other files to look the same
| final List<ConstraintViolation> violations = super.validate(); | ||
| return violations; | ||
| } | ||
|
|
There was a problem hiding this comment.
We don't need an empty line here.
| /** | ||
| * Returns {@code true} if the field has required attribute or validation is strict. | ||
| */ | ||
| protected boolean isRequiredField() { |
There was a problem hiding this comment.
Please have result variable here.
| } | ||
|
|
||
| /** | ||
| * Boolean field can't be determined if it set or not, because protobuf 'false' value and 'no value' are the same |
There was a problem hiding this comment.
Please have dot at the end of the sentence. While we are here, I'd say that it would be better to say that
”In Protobuf there is no way to tell if the value is {@code false} or was not set.”
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
Please change the Javadoc per my recent comment. In the rest LGTM.
No description provided.