Hi,
Thanks for the feedback.
I've made most of the suggested changes and will check in the code shortly. A
lot of your suggestions also apply to the existing mx validators.
> public static const: ERROR_CODE_WRONG_FORMAT:String = "wrongFormat";
Changed but you might want to note that the existing mx validators have these
hard coded.
>> Could this condition not be determined before getting to the start of the
>> loop (inner loop)
It was needed to be in the loop so that any formatting errors can be caught and
displayed.
> DECIMAL_DIGITS.indexOf(char) == -1
Again existing mx validators are written in this style but I like the
suggestion as it makes for more readable code.
It would probably make sense to move these methods up to the base class at some
point.
> combining the above two suggestions I can then glance at the
> validatingPostcode function and see quite quickly that it:
>
> for each format
> validates each character in the postcode
> holds the error
> converts the errors to results
I've made it close to that.
> There seems to be the opportunity to reduce the code here, as its seems to be
> 3 very similar blocks of code, could you extract this somewhere else and pass
> in the differences ?
Perhaps but I left it as it is. IMO changing actually adds more code and
doesn't really make it any easier to understand.
> private function getPreferredError():Object
> {
> errors.sortOn(ERROR_COUNT_FIELD, Array.NUMERIC);
> return errors[0];
> }
Again IMO replacing a one line of code with a function call (even a descriptive
one) doesn't make the code any more or less understandable.
> note: not that important, but also noticed a very minor inconsistency in the
> naming convention of the error type vs the errorCode and errorMessage.
Again just based on what existing mx validators had. :-)
Thanks,
Justin
