Fix a bug with the postal code validator where a pattern with a "|" can match any part of a postal …

Fix a bug with the postal code validator where a pattern with a "|" can match any part of a postal code, when it is intended to match only the prefix.

[lararennie, 2014-06-06 11:14:26]

https://codereview.appspot.com/103180043/diff/20001/src/address_input_helper.cc
File src/address_input_helper.cc (right):

https://codereview.appspot.com/103180043/diff/20001/src/address_input_helper....
src/address_input_helper.cc:139: // TODO: Modify the rule class to store and
expose this directly to make
Can you do the TODO instead? It shouldn't be major, and this is getting yucky
now.

[davinci_google, 2014-06-13 08:51:24]

On 2014/06/06 11:14:26, lararennie wrote:
>
https://codereview.appspot.com/103180043/diff/20001/src/address_input_helper.cc
> File src/address_input_helper.cc (right):
> 
>
https://codereview.appspot.com/103180043/diff/20001/src/address_input_helper....
> src/address_input_helper.cc:139: // TODO: Modify the rule class to store and
> expose this directly to make
> Can you do the TODO instead? It shouldn't be major, and this is getting yucky
> now.

I have exposed a "GetDefaultPostalCode" in the API.

[lararennie, 2014-06-13 09:17:23]

https://codereview.appspot.com/103180043/diff/40001/cpp/src/address_input_hel...
File cpp/src/address_input_helper.cc (right):

https://codereview.appspot.com/103180043/diff/40001/cpp/src/address_input_hel...
cpp/src/address_input_helper.cc:130: if (!default_postal_code.empty()) {
You could just do this unconditionally, setting an empty string to an empty
string shouldn't be expensive?

https://codereview.appspot.com/103180043/diff/40001/cpp/src/rule.h
File cpp/src/rule.h (right):

https://codereview.appspot.com/103180043/diff/40001/cpp/src/rule.h#newcode102
cpp/src/rule.h:102: const std::string& GetDefaultPostalCode() const { return
default_postal_code_; }
I think "default" is misleading here.

I'd call it GetSolePostalCode or something - because the point is, it's the only
postal code for this country.

[davinci_google, 2014-06-13 14:42:55]

https://codereview.appspot.com/103180043/diff/40001/cpp/src/address_input_hel...
File cpp/src/address_input_helper.cc (right):

https://codereview.appspot.com/103180043/diff/40001/cpp/src/address_input_hel...
cpp/src/address_input_helper.cc:130: if (!default_postal_code.empty()) {
On 2014/06/13 09:17:23, lararennie wrote:
> You could just do this unconditionally, setting an empty string to an empty
> string shouldn't be expensive?

Done.

https://codereview.appspot.com/103180043/diff/40001/cpp/src/rule.h
File cpp/src/rule.h (right):

https://codereview.appspot.com/103180043/diff/40001/cpp/src/rule.h#newcode102
cpp/src/rule.h:102: const std::string& GetDefaultPostalCode() const { return
default_postal_code_; }
On 2014/06/13 09:17:23, lararennie wrote:
> I think "default" is misleading here.
> 
> I'd call it GetSolePostalCode or something - because the point is, it's the
only
> postal code for this country.

Done.

[lararennie, 2014-06-13 14:56:32]

LGTM

https://codereview.appspot.com/103180043/diff/80001/cpp/src/rule.cc
File cpp/src/rule.cc (right):

https://codereview.appspot.com/103180043/diff/80001/cpp/src/rule.cc#newcode232
cpp/src/rule.cc:232: // If the "zip" field is not a regular expression, then it
is the sole
I think a unit test would also be worthwhile.

[davinci_google, 2014-06-13 15:57:45]

On 2014/06/13 14:56:32, lararennie wrote:
> LGTM
> 
> https://codereview.appspot.com/103180043/diff/80001/cpp/src/rule.cc
> File cpp/src/rule.cc (right):
> 
> https://codereview.appspot.com/103180043/diff/80001/cpp/src/rule.cc#newcode232
> cpp/src/rule.cc:232: // If the "zip" field is not a regular expression, then
it
> is the sole
> I think a unit test would also be worthwhile.

Done.