better way to do assignment and check result

Question

I have to use String.scan function, which returns empty array if there is no match.

I wanted to assign a variable with the scan function and check it there is a match, but unfortunately I cannot do that because it won't return nil or false on no match.

I wanted to do this (1 line):

if ip = str.scan(/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/)
  ...
  #use ip
end

but because it won't return nil on no match I must do:

ip_match = str.scan(/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/)
unless ip_match.empty?
  #use ip
end

Is there some more elegant way to write this - to be able to do assignment and empty check at the same time or some other way to beautify the code?

Thanks

Your ip validator seems to be week check out http://stackoverflow.com/questions/3756184/rails-3-validate-ip-string — Pritesh Jain, Sep 05 '12 at 07:41

score 2 · Answer 1 · answered Sep 05 '12 at 07:03

2

Since scan returns an array, and even if you are sure there would be only one result, you could do this.

str.scan(/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/).each do |ip|
  #use ip
end

answered Sep 05 '12 at 07:03

oldergod

15,033
7
62
88

score 1 · Answer 2 · answered Sep 05 '12 at 07:09

There's a difference between elegant and cryptic or "concise".

In Perl you'll often see people write something equivalent to:

if (!(ip = str.scan(/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/)).empty?)

It's a bit more concise, terse, tight, whatever you want to call it. It also leads to maintenance issues because of the = (equate) vs. what should normally be an equality test. If the code is passed to someone who doesn't understand the logic, they might mistakenly "correct" that, and then break the code.

In Ruby it's idiomatic to not use equate in a conditional test, because of the maintenance issue, and instead use the assignment followed by a test. It's clearer code.

Personally, I prefer to not use unless in that sort of situation. It's an ongoing discussion whether unless helps generate more understandable code; I prefer if (!ip_match.empty?) because it reads more like we'd normally talk -- I seldom start a statement with unless in conversation. Your mileage might vary.

score 1 · Answer 3 · edited May 23 '17 at 11:56

I would preferably do something like this using String helper match

ip_validator = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/

# match return nil if no match

if str.match ip_validator 
  # blah blah blah.....
end

help me keep code dry and clean. May be this is not the most elegant , looking for others if any :)

Your ip_validator regex seems to be week check this out Rails 3: Validate IP String

better way to do assignment and check result

3 Answers3