TRUNK-5564:PersonAddressValidator - Throws exception instead of logging #3041
Conversation
| @@ -64,7 +64,7 @@ public void validate(Object object, Errors errors) { | |||
| } | |||
|
|
|||
| if (object == null) { | |||
| throw new IllegalArgumentException("The personAddress object should not be null"); | |||
| errors.reject("error.Address"); | |||
There was a problem hiding this comment.
according to the needs of your ticket,i see you need to use some logging and cannot see it here
There was a problem hiding this comment.
HI @jwnasambu
Julie just use the logger defined in the class scope like below
if (obj == null) { log.error("Address object contains null value (in youre words)"); return; }
|
@Ayesh I have made the changes. Thanks |
| @@ -64,7 +64,8 @@ public void validate(Object object, Errors errors) { | |||
| } | |||
|
|
|||
| if (object == null) { | |||
| throw new IllegalArgumentException("The personAddress object should not be null"); | |||
| log.error("Address object contains null value"); | |||
| return; | |||
There was a problem hiding this comment.
do we really need the return i see your method is a void!!
There was a problem hiding this comment.
It's probably needed. Previously the code would not continue in this method because of the throw, this change now logs and returns instead. Thus, the explicit "return;" is needed to exit the method early; without it execution would continue in the function and a NullPointerException would likely follow.
|
Can you follow the same pattern used by other validators? https://github.com/openmrs/openmrs-core/tree/master/api/src/main/java/org/openmrs/validator |
|
The ticket says not to throw the exception. For example this validator does the same as @jwamalwa does for this validator and it's good to have a logging message for reference as well so I don't see anything to change here : |
|
@Akayeshmantha did you look at the validators in the link that i gave? |
|
Hmm yes, @dkayiwa . The one I posted in the previous comment is also a validator in the same link |
|
@Akayeshmantha how many validators did you look at? |
|
@dkayiwa, @Akayeshmantha I have made some changes kindly review my PR and feel free to advice me. |
|
@jwnasambu did you take a look at the validators that i pointed to? |
|
@dkayiwa yes I did only that they are several and am still going through to understand. |
|
Hi @dkayiwa Sure will go through |
Description of what I changed
Issue I worked on
https://issues.openmrs.org/browse/TRUNK-5564
I coded the PersonAddressValidator in a way that avoids throwing exceptions