incubator-flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Heidegger ...@leichtgewicht.at>
Subject Re: [Proposal] Add PostCodeValidator and PostCodeFormatter to the SDK
Date Sat, 10 Mar 2012 01:18:17 GMT
On 10/03/2012 09:03, Justin Mclean wrote:
> Couldn't find an instance of this can you point out which file?
Sorry, my fault, was browsing old version.

> I don't see it as an issue as the alternative would involve 3 if 
> statements and an extra variable.

Couldn't you just use var invalidFormat: Number = 0 or 1 ?

> I can't find any instances of that either. I used the "local" bracket style with the
exception of single line if statements which was discussed (briefly) on the list the other
week.
>
> if ()
>    doSomething();
> else
>    doSomethingElse();
>
> while compact without braces can lead to errors when modified at a later date.
PostCodeValidator - Line 560

>
>> *) You mix "brace in new line" (left) with "brace in current line" (right), afaik
its in the current line.
> Again I can't find an instance of this in the formatter or validator classes. I find
the  brace on current line more compact and have used that style in the unit tests.
I think there is a definite standard on that in the code guidelines of 
flex. however:
  PostCodeValidator - 499 & 513 just as one example


>> *) You have string checks like "if(value)" this also checks on "" but that is actually
a valid "format", right?
> I had considered "" as a invalid format as it's not particularly interesting. I'll double
check the unit tests around this.
It would be good to add that case to the documentation.

>
>> *) if() { return ... } return; is less good readable compared to if() { return }
else {return} again: Not sure what the style guide says.
> The SDK use both styles.
Personally I see bad sides in the one with the missing else ... however: 
again: What did the Flex Gui

> It may be nice to know all errors for formats at a later date. We're 
> only talking about very small arrays here.

I made the experience that programming for possibilities is a very bad 
idea. I am not necessarily talking about the performance (which it also 
affects) but
also the readability.

>> *) I have seen Japanese people writing their postal code with the unicode character
prefix: 〒 That is not an accepted Format
>  From what I've read the ITU only accepts A-Z and 0-9, but I certainly have no issue
in expanding the character range.

The question is how this util is defined (documentation) if it validates 
to the ITU standard it should mention that.
But if its about usability: Japanese people certain

>
>> *) Japanese do have wide format numbers that they also like to use in postal adresses:
0123456789
> I'll also look into this.

This is a big problem for all number validation in japan. Oh: They also 
have double width A-Z ABC...XYZ: try to build a search engine 
around that!

>> *) extraValidation is not sufficiently documented: No mention that the input can
be null, no mention that the output has to be
>> one of the error types that it can deal with. I would expect following signature:
extraValidation(postCode:String, baseField:String):Vector.<ValidationResult>;
> Will add more documentation. The existing mx class don't use arrays not vectors so changing
it to use vectors could cause confusion in people using multiple validator classes.
I see thanks! And thinking about it further: shouldn't the "result" have 
a error-weight? To be compared with other errors?

> The signature is in the same format as the other mx validate classes 
> and is familiar to people who use them. I agree your signature is 
> better but it's probably not the time to be changing them all. 

I do not see a need for this to be consistent, does it? Anyways: I will 
not a refactoring of this signature for Flex X somewhere.

>> *) I would put the static functions in their own function file to reduce dependency
hell.
> Again this is not the style of the currently validiators And I don't want to go down
the path of rewriting all of them just yet:-)
Same here: not sure this it is necessary to continue that trend :-)

>> *) suggestFormat Should be separated from the Validator as it is not "necessary"
to use the Validator (dependencies, dependencies)
> I think the connivence of having a single class is better. But happy to be convinced
otherwise. Can you give a code sample?

I attached three examples how it think it could work else. All have less 
dependencies and better performance.

> Fp 10.2 is the earliest version the current 4.6 SDK will compile with (and that's only
after I changed the build scripts)
Just because it compiles to this version doesn't mean it will work from 
this version on?! it is compilable with earlier SDK's as well ?!
>> *) @private
>> public function set formats
>>
>> What is this setter good for?
> For setting multiple formats: eg
> validator.formats = ["AN NAA", "ANN NAA", "AAN NAA", "ANA NAA", "AANN NAA", "AANA NAA"];
Then my question would be: Why is it private?

yours
Martin.

Mime
View raw message