sunlightlabs / opencongress

An open website for opening Congress.
http://www.opencongress.org
GNU General Public License v3.0
48 stars 16 forks source link

zip inference fix, zip4 zip4 zip4 zip4 zip4git diffgit diffgit diff #483

Closed crdunwel closed 9 years ago

crdunwel commented 9 years ago

merge me plz

plantfansam commented 9 years ago

:+1:

Cspeisman commented 9 years ago

Looks good. I'm a big fan of ruby's one line if statements.

  if zip4.match(/\d{4}/) and user_profile.zip_four.blank?
      user_profile.zip_four = zip4
  end

to be condensed into

  user_profile.zip_four = zip4 if zip4.match(/\d{4}/) && user_profile.zip_four.blank?

It could help cut down on the bulk. Obviously, it's just me being picky on readability and isn't a big deal at all. In the words of @handlers, there are plenty of other broken things to fix.

crdunwel commented 9 years ago

Yeah, one of the things I like best about ruby is the flexible in syntax. I like one liners as well and use them a lot. Typically I use them for boolean expressions with only one clause (and more-so with unless than if because that's more natural in english). For expressions with more than one clause, I usually break it into multiple lines so for greater reading clarity. Whether I use "or/and/not" or "||/&&/!" depends a lot on the length of each clause and the types of symbols in them. Typically the more mathy the evaluation, the more likely I will use the C derived symbols. Word methods I use the english.

Cspeisman commented 9 years ago

:shipit: