Refactor question

Hi,

I'm trying to refactor some code..

      if session[:image_subject_id].blank?         if @profile.image_subject_id.blank?           session[:image_subject_id] = nil         else           session[:image_subject_id] = @profile.image_subject_id         end       end

      if session[:image_subject_id].blank?         if @profile.image_book_id.blank?           session[:image_book_id] = nil         else           session[:image_book_id] = @profile.image_book_id         end       end        ...        ...        ...

Trying the following:

["image_subject_id", "image_book_id", "image_chapter_id", "image_section_id""image_subsection_id", "image_minisection_id"].each do

k>

          if session[":#{k.to_s}"].blank?             if @profile.k.blank?               session[":#{k.to_s}"] = nil             else               session[":#{k.to_s}"] = @profile.k             end           end end

Problem is @profile.k gives error NoMethodError: undefined method `k' for #<Profile:0x00000106da5e60>

k does evaluate to "image_subject_id", so not sure what is wrong...

Thanks

... Trying the following:

["image_subject_id", "image_book_id", "image_chapter_id", "image_section_id""image_subsection_id", "image_minisection_id"].each do >k>

          if session[":#{k.to_s}"].blank?

I don't see why you need .to_s as k should already be a string

            if @profile.k.blank?

Use send to invoke a method (actually send a message) using a string         if @profile.send(k).blank?

              session[":#{k.to_s}"] = nil             else               session[":#{k.to_s}"] = @profile.k             end           end end

However no doubt someone will come up with a much better way of accomplishing the whole thing.

Colin

Colin Law wrote in post #1145636:

lauantai, 10. toukokuuta 2014 11.21.24 UTC+3 Colin Law kirjoitti:

Trying the following:

[“image_subject_id”, “image_book_id”, “image_chapter_id”,

“image_section_id”“image_subsection_id”, “image_minisection_id”].each do

k>

      if session[":#{k.to_s}"].blank?

I don’t see why you need .to_s as k should already be a string

It doesn’t even matter, since the hash syntax within a string calls .to_s implicitly:

irb(main):026:0> “1234 #{:foo} 232”

=> “1234 foo 232”

Anyway, you won’t even need to change the symbols to strings:

[:image_subject_id, :image_book_id, :image_chapter_id, :image_section_id, :image_subsection_id, :image_minisection_id].each do

k>

      if session[k].blank? 
        if @profile.send(k).blank? 
          session[k] = nil 
        else 
          session[k] = @profile.send(k)
        end 
      end 

end

That said, do you even need to check for the corner cases like that? Since you’re talking about ids, I would assume they can only be nil or a valid id. So would this be sufficient (inside the loop):

session[k] ||= @profile.send(k)

If you do have to e.g. take empty strings (with which .blank? is true) into account, then something like this perhaps?

if session[k].blank?

session[k] = (val = @profile.send(k)).blank? ? nil : val end

//jarkko