Skip to content
Browse files

Code review from @mathie

  • Loading branch information...
1 parent 5dab7b1 commit f36731c68a75cbc21a56e6009722e31db3e12192 @lenary lenary committed Jun 21, 2012
Showing with 36 additions and 33 deletions.
  1. +7 −6 lib/sage_pay/server/address.rb
  2. +29 −27 lib/sage_pay/validators.rb
View
13 lib/sage_pay/server/address.rb
@@ -2,6 +2,7 @@ module SagePay
module Server
class Address
include ActiveModel::Validations
+ include SagePay::Validators
attr_accessor :first_names, :surname, :address_1, :address_2, :city,
:post_code, :country, :state, :phone
@@ -10,10 +11,10 @@ class Address
# FIXME: This regexp isn't correctly matching accented characters. I
# think it's a Ruby version issue so I'm punting for the moment.
- validates_format_of :first_names, :surname, :with => NameFormat
- validates_format_of :address_1, :address_2, :city, :with => AddressFormat
- validates_format_of :post_code, :with => PostCodeFormat
- validates_format_of :phone, :with => PhoneFormat
+ validates_format_of :first_names, :surname, :with => NAME_FORMAT
+ validates_format_of :address_1, :address_2, :city, :with => ADDRESS_FORMAT
+ validates_format_of :post_code, :with => POST_CODE_FORMAT
+ validates_format_of :phone, :with => PHONE_FORMAT
validates_length_of :first_names, :surname, :maximum => 20
validates_length_of :address_1, :address_2, :maximum => 100
@@ -26,8 +27,8 @@ class Address
# contains two-character strings, so this validation has no real win.
# validates_length_of :country, :state, :maximum => 2
- validates_inclusion_of :state, :in => USStateOptions, :allow_blank => true, :message => "is not a US state"
- validates_inclusion_of :country, :in => CountryOptions, :allow_blank => true, :message => "is not an ISO3166-1 country code"
+ validates_inclusion_of :state, :in => US_STATE_OPTIONS, :allow_blank => true, :message => "is not a US state"
+ validates_inclusion_of :country, :in => COUNTRY_OPTIONS, :allow_blank => true, :message => "is not an ISO3166-1 country code"
# The state's presence is required if the country is the US, and
# verboten otherwise.
View
56 lib/sage_pay/validators.rb
@@ -1,34 +1,36 @@
module SagePay
- VendorNameFormat = /\A[A-Za-z0-9\-_]*\z/.freeze
- VendorTxCodeFormat = /\A[A-Za-z0-9\-_\.{}]*\z/.freeze
+ module Validators
+ VENDOR_NAME_FORMAT = /\A[A-Za-z0-9\-_]*\z/.freeze
+ VENDOR_TX_CODE_FORMAT = /\A[A-Za-z0-9\-_\.{}]*\z/.freeze
- # The following lack the requisite accented character matchers
- NameFormat = /\A[[:alpha:] \\\/&'\.\-]*\z/.freeze
- AddressFormat = /\A[[:alnum:][:space:]\+\\\/&'\.:,\(\)\-]*\z/.freeze
+ # The following lack the requisite accented character matchers
+ NAME_FORMAT = /\A[[:alpha:] \\\/&'\.\-]*\z/.freeze
+ ADDRESS_FORMAT = /\A[[:alnum:][:space:]\+\\\/&'\.:,\(\)\-]*\z/.freeze
- PostCodeFormat = /\A[[:alnum:] -]*\z/.freeze
- PhoneFormat = /\A[[:alnum:] \+\(\)-]*\z/.freeze
+ POST_CODE_FORMAT = /\A[[:alnum:] -]*\z/.freeze
+ PHONE_FORMAT = /\A[[:alnum:] \+\(\)-]*\z/.freeze
- # Basic
- EmailFormat = /@/.freeze
+ # Basic
+ EMAIL_FORMAT = /@/.freeze
- CountryOptions = %w{AF AX AL DZ AS AD AO AI AQ AG AR AM AW AU AT AZ BS BH BD
- BB BY BE BZ BJ BM BT BO BQ BA BW BV BR IO BN BG BF BI KH
- CM CA CV KY CF TD CL CN CX CC CO KM CG CD CK CR CI HR CU
- CW CY CZ DK DJ DM DO EC EG SV GQ ER EE ET FK FO FJ FI FR
- GF PF TF GA GM GE DE GH GI GR GL GD GP GU GT GG GN GW GY
- HT HM VA HN HK HU IS IN ID IR IQ IE IM IL IT JM JP JE JO
- KZ KE KI KP KR KW KG LA LV LB LS LR LY LI LT LU MO MK MG
- MW MY MV ML MT MH MQ MR MU YT MX FM MD MC MN ME MS MA MZ
- MM NA NR NP NL NC NZ NI NE NG NU NF MP NO OM PK PW PS PA
- PG PY PE PH PN PL PT PR QA RE RO RU RW BL SH KN LC MF PM
- VC WS SM ST SA SN RS SC SL SG SX SK SI SB SO ZA GS SS ES
- LK SD SR SJ SZ SE CH SY TW TJ TZ TH TL TG TK TO TT TN TR
- TM TC TV UG UA AE GB US UM UY UZ VU VE VN VG VI WF EH YE
- ZM ZW}.freeze
+ COUNTRY_OPTIONS = %w{AF AX AL DZ AS AD AO AI AQ AG AR AM AW AU AT AZ BS BH
+ BD BB BY BE BZ BJ BM BT BO BQ BA BW BV BR IO BN BG BF
+ BI KH CM CA CV KY CF TD CL CN CX CC CO KM CG CD CK CR
+ CI HR CU CW CY CZ DK DJ DM DO EC EG SV GQ ER EE ET FK
+ FO FJ FI FR GF PF TF GA GM GE DE GH GI GR GL GD GP GU
+ GT GG GN GW GY HT HM VA HN HK HU IS IN ID IR IQ IE IM
+ IL IT JM JP JE JO KZ KE KI KP KR KW KG LA LV LB LS LR
+ LY LI LT LU MO MK MG MW MY MV ML MT MH MQ MR MU YT MX
+ FM MD MC MN ME MS MA MZ MM NA NR NP NL NC NZ NI NE NG
+ NU NF MP NO OM PK PW PS PA PG PY PE PH PN PL PT PR QA
+ RE RO RU RW BL SH KN LC MF PM VC WS SM ST SA SN RS SC
+ SL SG SX SK SI SB SO ZA GS SS ES LK SD SR SJ SZ SE CH
+ SY TW TJ TZ TH TL TG TK TO TT TN TR TM TC TV UG UA AE
+ GB US UM UY UZ VU VE VN VG VI WF EH YE ZM ZW}.freeze
- USStateOptions = %w{AL AK AZ AR CA CO CT DE DC FL GA HI ID IL IN IA KS KY LA
- ME MD MA MI MN MS MO MT NE NV NH NJ NM NY NC ND OH OK OR
- PA RI SC SD TN TX UT VT VA WA WV WI WY AS GU MP PR VI FM
- MH PW AA AE AP}.freeze
+ US_STATE_OPTIONS = %w{AL AK AZ AR CA CO CT DE DC FL GA HI ID IL IN IA KS KY
+ LA ME MD MA MI MN MS MO MT NE NV NH NJ NM NY NC ND OH
+ OK OR PA RI SC SD TN TX UT VT VA WA WV WI WY AS GU MP
+ PR VI FM MH PW AA AE AP}.freeze
+ end
end

0 comments on commit f36731c

Please sign in to comment.
Something went wrong with that request. Please try again.