CODEWITHSUNDEEP
ruby
Tom
Tom

Reputation: 1105

Ruby If statement refactor

I'm coming back to Ruby after a long time away.

I've written the following code which is functional:

def generate_address_record(data)
 address = FM[:fap_address].build do |a|
        if data['Line 1'].blank?
          a.unstructured_address.line1 = nil
        else
          a.unstructured_address.line1 = data['Line 1']
        end

        if data['Line 2'].blank?
          a.unstructured_address.line2 = nil
        else
          a.unstructured_address.line2 = data['Line 2']
        end

        if data['Line 3'].blank?
          a.unstructured_address.line3 = nil
        else
          a.unstructured_address.line3 = data['Line 3']
        end

        if data['Line 4'].blank?
          a.unstructured_address.line4 = nil
        else
          a.unstructured_address.line4 = data['Line 4']
        end

        if data['Line 5'].blank?
          a.unstructured_address.line5 = nil
        else
          a.unstructured_address.line5 = data['Line 5']
        end

        if data['Postcode'].blank?
          a.unstructured_address.postcode = nil
        else
          a.unstructured_address.postcode = data['Postcode']
        end
  end
end

Is there a way this can be re-written in a 'nicer' way in one loop so that I don't need all of these individual if statements.

Any advice would be greatly appreciated.

Upvotes: 1

Views: 97

Answers (4)

Fabio
Fabio

Reputation: 32445

If data contains only expected values, you can convert key to the method name.

def generate_address_record(data)
  address = FM[:fap_address].build do |a|
      data.each do |key, value|
          name = key.gsub(/[[:space:]]/, '').downcase
          a.unstructured_address.public_send("#{name}=", value.presence)
      end
  end
end

Be careful(or don't use this approach) when hash keys are coming from outside of the application or other uncontrolled environment.

Upvotes: 2

spickermann
spickermann

Reputation: 106882

I propose this combination of the various already posted solutions because I think it has a good balance between shortness and readability:

def generate_address_record(data)
  address = FM[:fap_address].build do |a|
    a.unstructured_address.line1    = data['Line 1'].presence
    a.unstructured_address.line2    = data['Line 2'].presence
    a.unstructured_address.line3    = data['Line 3'].presence
    a.unstructured_address.line4    = data['Line 4'].presence
    a.unstructured_address.line5    = data['Line 5'].presence
    a.unstructured_address.postcode = data['Postcode'].presence
  end
end

Upvotes: 2

mrzasa
mrzasa

Reputation: 23327

Yup, you can use #presence (I'm assuming you're using Rails):

a.unstructured_address.line1 = data['Line 1'].presence

#presence behaviour:

''.presence
# => nil
nil.presence
# => nil
'a'.presence
# => "a"
false.presence
# => nil

Upvotes: 2

max pleaner
max pleaner

Reputation: 26768

One pattern I find usefull is to make a hash and then iterate over it:

def generate_address_record(data)
   address = FM[:fap_address].build do |a|
     {
       "Line 1" =>  :line1, 
       "Line 2" =>  :line2,
       "Line 3" =>  :line3,
       "Line 4" =>  :line4,
       "Line 5" =>  :line5,
       "Postcode" => :postcode
     }.each do |key, accessor|
       if data[key].blank?
         a.unstructured_address.send(accessor) = nil
       else
         a.unstructured_address.send(:"#{accessor}=") = data[key]
       end
     end
  end
end

You can also use this with presence as mrzasa shared

Upvotes: 0

Related Questions