Rails: Should partials be aware of instance variables?

Question

Ryan Bates' nifty_scaffolding, for example, does this

edit.html.erb

<%= render :partial => 'form' %>

new.html.erb

<%= render :partial => 'form' %>

_form.html.erb

<%= form_for @some_object_defined_in_action %>

That hidden state makes me feel uncomfortable, so I usually like to do this

edit.html.erb

<%= render :partial => 'form', :locals => { :object => @my_object } %>

_form.html.erb

<%= form_for object %>

So which is better: a) having partials access instance variables or b) passing a partial all the variables it needs?

I've been opting for b) as of late, but I did run into a little pickle:

some_action.html.erb

<% @dad.sons.each do |a_son| %>
<%= render :partial => 'partial', :locals => { :son => a_son } %>
<% end %>

_partial.html.erb

The son's name is <%= son.name %>
The dad's name is <%= son.dad.name %>

son.dad makes a database call to fetch the dad! So I would either have to access @dad, which would be going back to a) having partials access instance variables or I would have to pass @dad in locals, changing render :partial to <%= render :partial => 'partial', :locals => { :dad => @dad, :son => a_son } %>, and for some reason passing a bunch of vars to my partial makes me feel uncomfortable. Maybe others feel this way as well.

Hopefully that made some sense. Looking for some insight into this whole thing... Thanks!

Solution 1

In recent versions of Rails it is quite a bit easier to render partials and pass locals to them. Instead of this.

<%= render :partial => 'form', :locals => { :item => @item } %>

You can do this.

<%= render 'form', :item => @item %>

I don't do this in the Nifty Scaffold generator to keep backwards compatibility, but I'll change this in a future release.

As for whether it's acceptable to use instance variables in partials. I think it is. In all practicality, what is the downside? Certainly things can get out of hand if you aren't consistent, but I like to apply these guidelines.

1. Never create an instance variable just to share it between partials. Usually this means you will only be sharing the controller resource object.

2. If the partial is the same name as the resource, pass it as a local with <%= render @item %>.

3. If the partial will be shared across multiple controllers then only use locals.

This is what works well for me anyway.

Bonus tip: if you find yourself passing in a lot of locals into a partial and you want some of them to be optional, create a helper method which renders the partial. Then always go through the helper method so you can make a clean interface with optional args for rendering the partial.

Solution 2

Using @instance_variables in partials is bad design.

Using instance variable in partials works, but it can make it harder to maintain applications if changes are ever needed.

The downside of using instance variables in partials is that you create a dependency in the partial to something outside the partial's scope (coupling). This makes the partial harder to reuse, and can force changes in several parts of the application when you want to make a change in one part.

Partials that use instance variables:

• must change when the instance variable in any controller that uses the partial changes either the instance variable name or its type or data structure
• cause all controller actions that use the partial to change in the same way at the same time when there are changes to how the instance variable is used
• discourage reuse, as they can only easily be reused in actions that set up instance variables with the same name and data

Instead, pass locals to the partials:

<%= render 'reusable_partial', :item => @item %>

Now, because the partial only references item and not @item, the action that renders the view that renders the reusable_partial is free to change without affecting the reusable_partial and the other actions/views that render it:

<%= render 'reusable_partial', :item => @other_object.item %>

Also, this can be reused in contexts where there is no @item:

<%= render 'reusable_partial', :item => @duck %>

If my @duck changes in the future and no longer quacks like reusable_partial expects it to (the object's interface changes), I can also use an adapter to pass in the kind of item that reusable_partial expects:

<%= render 'reusable_partial', :item => itemlike_duck(@duck) %>

Always?

There are plenty of situations where you probably don't need de-coupled partials like this, and it's easier in the short run to use an instance variable. However, it's hard to predict the future needs of your application.

As such, this makes for good general practice while having relatively low cost.

Solution 3

You can have it both ways. At the top of your partial:

<% item ||= @item %>

That way, it works with or without passing the local variable, providing a sane default, but not inhibiting alternate usage of the partial.

Solution 4

I vote for a) for a very specific reason -- DRY! If you start passing a variable like that, the next thing you know it's a mess. Let's say you need to change the way your variable is named or something else about it. You'll need to go to ALL your views and change them instead of ONE partial.

Also, if you change your partial it will change on all your views, so you'll need to know which views are used. A proper IDE should be able to help you with that, but I also like having a small comment section at the top of the view where I just mention where it's used and why. This helps another programmer and it helps you to remember in case you need to come back to a partial and modify. But the whole point of the partial is to call it WITHOUT having to pass anything from the view, so that you don't have to modify all places where partial is called from if that variable changes somehow.

Ultimately this is a design choice, and to be honest unless you are running a facebook the extra lookup you do is not that big of a deal, but it's just not very DRY.

P.S.: Just thought about it. You can actually abstract the way you call partial in a helper method, so then if the way you call your partial needs to change, you just need to modify one place.

View more solutions

Comments

[RocketR]

When something's not DRY, it's still not a reason to spread global vars all around the code. Otherwise, all the functions would become parameterless.

[dubilla]

+1 for "3. If the partial will be shared across multiple controllers then only use locals."

[illusionist]

completely agree with you @nilbus. felt the same while working around with partials specially reusable ones

[Derek Prior]

This makes the coupling totally unclear. The partial now has two contracts.

[ahnbizcad]

ew for the new syntax. too ambiguous.

[ahnbizcad]

so we know what not to do. don't pass local variables around partial to partial. don't use global variables. So a good-practice solution would be... helpers?

[Magne]

"However, it's hard to predict the future needs of your application." -> Thus, I'd add locals when needed, instead of prematurely optimising this. The nice thing then, besides less code (points of failure), is that when you see a local passed into a partial, you know immediately that it is used by other controllers too.

[Magne]

I'd also argue that using instance variables instead of passing locals is more the rails way of relying on convention instead of configuration. Specifying locals when the partial is only used by the same controller (which btw ought to have the instance variable set only one place: in a common before_action method), represents unnecessary configuration/specificity.