Actually I think I should return immediately after null checks. That way I
don't have to add all these guard clauses.
I will change the code.


On Fri, Mar 14, 2014 at 12:12 AM, wluyima <notifications@github.com> wrote:

> In api/src/main/java/org/openmrs/validator/DrugOrderValidator.java:
>
> > @@ -75,6 +81,29 @@ public void validate(Object obj, Errors errors) {
> >  					
> >  				}
> >  			}
> > +			if (order.getDosingType() != null) {
>
> A good practice we use when writing these validators, is that after
> checking for required fields is to check if there are any validation errors
> at that point and return immediately, i guess this is why i assumed it
> halts it but looks like this validator was written validator. It doesn't
> make sense to do further validation on the same field that was already
> found to be null e.g:
>
> ValidationUtils.rejectIfEmpty(errors, "dose", "mesasge");
> ValidationUtils.rejectIfEmpty(errors, "startDate", "mesasge");
> if(errors.hasErrors())
> return;
>
> //At this point i know some fields can't be null
> //the check if startDate is not in the future without checking for null
>
> But i guess it is okay if you prefer to write it that way.
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/openmrs/openmrs-core/pull/774/files#r10579370>
> .
>