incubator-flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Mclean <jus...@classsoftware.com>
Subject Re: [Proposal] Add PostCodeValidator and PostCodeFormatter to the SDK
Date Fri, 09 Mar 2012 23:10:47 GMT
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


Mime
View raw message