# Refactor question

**URL:** <https://discuss.rubyonrails.org/t/refactor-question/68981>\
**Category:** rubyonrails-talk\
**Created:** [May 10, 2014, 2:46am UTC](https://discuss.rubyonrails.org/t/refactor-question/68981 "2014-05-10T02:46:24Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![11155](https://avatars.discourse-cdn.com/v4/letter/1/d26b3c/32.png) [@11155](https://discuss.rubyonrails.org/u/11155)\
**Post date:** [May 10, 2014, 2:46am UTC](https://discuss.rubyonrails.org/t/refactor-question/68981/1 "2014-05-10T02:46:24Z")

</div>

Hi,

I'm trying to refactor some code..

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if session[:image\_subject\_id].blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if @profile.image\_subject\_id.blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[:image\_subject\_id] = nil &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[:image\_subject\_id] = @profile.image\_subject\_id &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if session[:image\_subject\_id].blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if @profile.image\_book\_id.blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[:image\_book\_id] = nil &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[:image\_book\_id] = @profile.image\_book\_id &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;... &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;... &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;...

Trying the following:

["image\_subject\_id", "image\_book\_id", "image\_chapter\_id", "image\_section\_id""image\_subsection\_id", "image\_minisection\_id"].each do

> k\>

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if session[":#{k.to\_s}"].blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if @profile.k.blank? &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[":#{k.to\_s}"] = nil &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[":#{k.to\_s}"] = @profile.k &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;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

---

<div class="post-metadata">

**Author:** ![Colin\_Law](https://avatars.discourse-cdn.com/v4/letter/c/b9e5f3/32.png) [@Colin\_Law](https://discuss.rubyonrails.org/u/Colin_Law)\
**Post date:** [May 10, 2014, 8:21am UTC](https://discuss.rubyonrails.org/t/refactor-question/68981/2 "2014-05-10T08:21:24Z")

</div>

> ... Trying the following:
> 
> ["image\_subject\_id", "image\_book\_id", "image\_chapter\_id", "image\_section\_id""image\_subsection\_id", "image\_minisection\_id"].each do \>k\>
> 
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if session[":#{k.to\_s}"].blank?

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

> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if @profile.k.blank?

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

> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[":#{k.to\_s}"] = nil &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;else &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;session[":#{k.to\_s}"] = @profile.k &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;end end

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

Colin

---

<div class="post-metadata">

**Author:** ![11155](https://avatars.discourse-cdn.com/v4/letter/1/d26b3c/32.png) [@11155](https://discuss.rubyonrails.org/u/11155)\
**Post date:** [May 10, 2014, 11:01am UTC](https://discuss.rubyonrails.org/t/refactor-question/68981/3 "2014-05-10T11:01:51Z")

</div>

Colin Law wrote in post #1145636:

---

<div class="post-metadata">

**Author:** ![Jarkko\_Laine1](https://avatars.discourse-cdn.com/v4/letter/j/8edcca/32.png) [@Jarkko\_Laine1](https://discuss.rubyonrails.org/u/Jarkko_Laine1)\
**Post date:** [May 10, 2014, 6:11pm UTC](https://discuss.rubyonrails.org/t/refactor-question/68981/4 "2014-05-10T18:11:14Z")

</div>

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