CODEWITHSUNDEEP
ruby-on-railsruby
krunal shah
krunal shah

Reputation: 16339

Better way to write if condition in ruby

if args.size == 5
  value_for,alt_currency_id,amount,exchange_rate_code,tran_dt = args
else
  value_for,alt_currency_id,amount,exchange_rate_code,year_no,period_no = args
end

Any Better way to write this condition ??

Upvotes: 1

Views: 1253

Answers (7)

Chubas
Chubas

Reputation: 18053

Definitely is a code smell, specially since the variable is called args. If you're passing all these arguments as optional values, the best approach is make the variable arguments into a hash.

def whatever(value_for, alt_currency_id, amount, options = {})
  tran_dt = options[:tran_dt]
  year_no = options[:year_no]
  period_no = options[:period_no]
  ...
end

Upvotes: 2

gertas
gertas

Reputation: 17145

Are you processing commandline? Just leave as it is, for me it is most readable at first look :) Otherwise it may smell perlish. You simply see what is required set for 5 arguments or else. If these are not command line args I suggest introducing hash.

Upvotes: 0

Lars Haugseth
Lars Haugseth

Reputation: 14881

Here's one way of DRYing up your code a bit:

value_for, alt_currency_id, amount, exchange_rate_code, year_no, period_no = args
if period_no.nil?
  tran_dt = year_no
  year_no = nil # May or may not be needed, depending on later code
end

Upvotes: 1

glenn jackman
glenn jackman

Reputation: 247210

To strictly meet your requirements, I'd do this:

value_for, alt_currency_id, amount, exchange_rate_code = args.shift(4)
tran_dt, year_no, period_no = [nil, nil, nil] # or some sensible defaults
case args.size
when 1 then tran_dt = args.shift
when 2 then year_no, period_no = args.shift(2)
end

But this code has a smell to it. I'd look at redesigning how that method gets called.

Upvotes: 2

Jim Garvin
Jim Garvin

Reputation: 4906

Perhaps assign period_no to nil by default, and use that to determine which argument set you are working with:

def process_record(value_for, alt_currency_id, amount, exchange_rate_code, tran_dt, period_no=nil)
  year_no = period_no ? tran_dt : nil
  puts "tran_dt: #{tran_dt.inspect}"
  puts "year_no: #{year_no.inspect}"
  puts "period_no: #{period_no.inspect}"
end

process_record(:foo, :bar, :baz, :buz, Time.now)
# Output:
#
# tran_dt: Mon Sep 13 15:52:54 -0400 2010
# year_no: nil
# period_no: nil

process_record(:foo, :bar, :baz, :buz, 2010, 1)
# Output:
#
# tran_dt: 2010
# year_no: 2010
# period_no: 1

Upvotes: 1

cam
cam

Reputation: 14222

I would just skip the condition entirely. If you don't have the fifth argument, period_no will simply be nil.

If period_no needed to be set to some default you could follow up with:

period_no ||= sane_default

Upvotes: 3

rivenate247
rivenate247

Reputation: 2101

Ruby has two ternary operators as well that I'm aware of

a = true  ? 'a' : 'b' #=> "a"  
b = false ? 'a' : 'b' #=> "b"

or

a = (true  && 'a') || b #=> "a"  
b = (false && 'a') || b #=> "b"

Upvotes: 0

Related Questions