0

I'm new to rails and ruby... How can i refactor such code, which import's from csv file data? Now i have such code

  if row[qnt].to_s != ""
    eqnt = /(\d+)/.match(row[qnt])[0].to_s
  else
    eqnt = 0
  end

I try something like

if row[qnt].present?
        eqnt = /(\d+)/.match(row[qnt])[0].to_s
      else
        eqnt = 0
      end

But is it equal, and also what else could i do to get code smaller?

Valdis Azamaris
  • 1,433
  • 5
  • 22
  • 47
  • `present?` documentation: http://api.rubyonrails.org/classes/Object.html#method-i-present-3F Some examples of present?: `[].present? #=> false` | `nil.present? #=> false` | `"".present? #=> false` | `" ".present? #=> false` (empty string) – MrYoshiji Jan 21 '13 at 19:43
  • But OP is already using `present?`, I think the point is that the OP wants to have so-called "elegant" code. – Dave Newton Jan 21 '13 at 19:45
  • No. Valdis said "I try something like [code] But is it equal, and also [blabla]". I assume Valdis is asking about `present?` and `.to_s != ""`. – MrYoshiji Jan 21 '13 at 19:50
  • He's asking if `present?` can be equated to `.to_s != ""` and also make the code cleaner and/smaller. That's what I get from this. – Leo Correa Jan 21 '13 at 19:59
  • @LeoCorrea you are right – Valdis Azamaris Jan 21 '13 at 20:00

3 Answers3

1

How about this?

row[qnt].present? ? eqnt = /(\d+)/.match(row[qnt])[0].to_s : eqnt = 0
Leo Correa
  • 19,131
  • 2
  • 53
  • 71
  • Ugh, two assignment statements where one would do, and IMO be easier to understand. – Dave Newton Jan 21 '13 at 19:47
  • @DaveNewton I agree with you, I wrote it without thinking much of the assignment and more about compressing code. To be honest, my first thought was to go with your second option as it's the most readable one but I figured since the OP wanted to make the code smaller. – Leo Correa Jan 21 '13 at 19:57
0

I'm not convinced the code gains readability by trying to compress it much further.

eqnt = row[qnt].present? ? /(\d+)/.match(row[qnt])[0].to_s : 0

Or

eqnt = 0
eqnt = /(\d+)/.match(row[qnt])[0].to_s if row[qnt].present?

Or

theRow = row[qnt]
eqnt = theRow.present? ? /(\d+)/.match(theRow).first.to_s : 0

Or better yet, extract this into a method, keep the mainline code clean, and isolate the logic.

I'm not psyched about eqnt ending up with different types, though, unless that's by design.

Dave Newton
  • 158,873
  • 26
  • 254
  • 302
0
eqnt = (/(\d+)/.match(row[qnt]) || [0])[0]
Casper
  • 33,403
  • 4
  • 84
  • 79