Sunday, August 26, 2007

Using anonymous functions inside functions

I was reading an article on Javascript and its use of first class functions. I'd quote it if I could find it again, but basically, it extolled the virtues of being able to create anonymous functions, blocks, and closures easily in Javascript. One of the things that it claimed was that creating local functions only used within a method makes the code much more readable. I was skeptical, but as I was refactoring this function, I thought I'd give it a shot even though I was coding in Ruby. Ruby also allows for the creation of first class functions.

It's just a helper function that generates a bookmark button--however, what gets generated depends on whether a user is logged in and whether a user had already bookmarked the page.
  def bookmark_button(page, user, indicator_id)
if page.bookmarked_by? session[:user]
"Bookmarked!"
elsif page.done_by? session[:user]
link_to_remote image_tag('heart_add.png', :height => 16, :width => 16) + " Bookmark again!",
:url => { :controller => :bookmark_list, :action => :add, :page_id => page.id },
:loading => "Element.toggle('#{indicator_id}')",
:complete => "Element.toggle('#{indicator_id}')"
else
link_to_remote image_tag('heart_add.png', :height => 16, :width => 16) + " Bookmark!",
:url => { :controller => :bookmark_list, :action => :add, :page_id => page.id },
:loading => "Element.toggle('#{indicator_id}')",
:complete => "Element.toggle('#{indicator_id}')"
end
end

Well, I thought it was pretty straightforward to read, though half of it was duplicated code. It wasn't very DRY. So instead of fudging around with another way to structure the control flow, I tried my hand at using blocks. I ended up with this:
  def bookmark_button(page, user, indicator_id)
bookmark_link = Proc.new { |label|
link_to_remote "#{image_tag('heart_add.png', :height => 16, :width => 16)} #{label}",
:url => { :controller => :bookmark_list,
:action => :add,
:page_id => page.id },
:loading => "Element.toggle('#{indicator_id}')",
:complete => "Element.toggle('#{indicator_id}')"
}

if page.bookmarked_by? session[:user]
"Doing this"
elsif page.done_by? session[:user]
bookmark_link.call("Bookmark again!")
else
bookmark_link.call("Bookmark!")
end
end

As you can see, I put the link_to_remote() code into a block, since the only difference between the two is the label. In addition, it is possible for the block to use variables that are in this method, but outside the block, even if the execution is outside the scope of this function, if the block is ever passed outside of bookmark_button. I know in javascript, this causes memory leaks in IE. I haven't heard that it's a problem in Ruby yet.

But in this case, I'm just creating a function inside a method to make things more readable and cleaner. I think for the most part, it worked pretty well. As long as it's not overdone (notice I didn't create a proc for the labels), it should make for more readable code, and you don't pollute the class namespace with methods that are only used in this one method. I should get use to this. tip!

5 comments:

  1. Hey, that's much DRYer, but you could go further. To really use an *anonymous* function, you could just use Proc.new{ code } and then append .call() onto it. And you could use the ternary operator inside the call()'s parens to determine the argument.

    i.e.

    Proc.new {|label| ...code using label... }.call(test ? "this label" : "that label")

    e.g.

    if page.bookmarked_by? session[:user]
    "Doing this"
    else
    Proc.new { |label|
    link_to_remote "#{image_tag('heart_add.png',
    :height => 16, :width => 16)} #{label}",
    :url => { :controller =>
    :bookmark_list,
    :action => :add,
    :page_id => page.id },
    :loading => "Element.toggle('#{indicator_id}')",
    :complete => "Element.toggle('#{indicator_id}')"
    }.call(page.done_by? session[:user] ?
    "Bookmark again!" : "Bookmark!")
    end


    Some things to point out about this construction:

    1.You never add to your namespace with a name for your proc.
    2. The proc becomes the center of attention, at least to human readers, and the argument passed to it (and the reasons for passing it), become secondary.
    3. It looks mighty weird at first, but actually it isn't much stranger than, say, sprintf, which also pushes the thing you're used to worrying about (what's the variable's value?) to the end of the line/block.

    mh

    ReplyDelete
  2. Daddy likes...!

    Huh, I didn't think of just mashing it all together. I think in this case, what you did makes a lot of sense--especially the part about how the proc becomes the center of attention, and what exactly gets displayed becomes secondary.

    Thanks for the tip.

    ReplyDelete
  3. Looks like someone got a new hammer, and everything is looking like a nail.

    I would have factored out the shared code like so:
    def bookmark_button(page, user, indicator_id)
    if page.bookmarked_by? session[:user]
    "Bookmarked!"
    else
    link_text = " Bookmark!"
    if page.done_by? session[:user]
    link_text = " Bookmark again!"
    end

    link_to_remote image_tag('heart_add.png', :height => 16, :width => 16) + link_text,
    :url => { :controller => :bookmark_list, :action => :add, :page_id => page.id },
    :loading => "Element.toggle('#{indicator_id}')",
    :complete => "Element.toggle('#{indicator_id}')"
    end
    end


    Am I missing something?

    ReplyDelete
  4. Hrm...John's comment gives me pause for thought. While I don't think using Proc is necessarily better or worthwhile in this case, it does put a different focus on the code.

    Which way is better should be which one that is clearer. Unfortunately, I think Which one is clearer probably depends on what you're use to.

    Recently, I've had a preference for functional style programming, and hence Michael's method looks perfectly ok to me.

    Perhaps this is a case of using a functional style when it doesn't have a clear advantage over imperative style--other than a different focus for the code.

    ReplyDelete
  5. John: Your point about simplicity is well taken. When one is dealing with actual working code, KISS is the acronym to remember. However, on a blog like Wilhelm's, code is often posted for exemplary purposes only, to illustrate an idea. Anonymous functions might be a better idea given more complicated circumstances, but who wants to post (or read) complicated code examples in a blog?

    ReplyDelete