CODEWITHSUNDEEP
ruby
all jazz
all jazz

Reputation: 2017

better way to do assignment and check result

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

Upvotes: 0

Views: 123

Answers (3)

Pritesh Jain
Pritesh Jain

Reputation: 9146

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

Upvotes: 1

the Tin Man
the Tin Man

Reputation: 160571

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.

Upvotes: 1

oldergod
oldergod

Reputation: 15010

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

Upvotes: 2

Related Questions