Coding Guidelines: Instance variables in views

Mo Morsi mmorsi at redhat.com
Thu Aug 16 18:06:04 UTC 2012


On 08/16/2012 11:05 AM, Matt Wagner wrote:
> Hi folks,
>
> I've noticed that we've been objecting to patches using instance
> variables to pass information to views, so I went to see our coding
> guidelines page[1] to read about why.
>
> It makes very clear that it's bad, but it doesn't really explain the
> right way. I'd hoped that the Coding Guidelines page would be more of a
> guide on how to do things the right way.
>
> I've really only found one blog post[2] that addresses the issue, and it
> doesn't really give a great example. It still uses an instance variable
> in the controller, but inside of a view it passes it as a local
> variable. It's more specific, sure, but it's not abundantly clear how
> that actually makes anything better, or why an instance variable from
> the controller to the view is okay but a view to a partial is not okay.
>
> Can someone with a more concrete understanding of the right way update
> the wiki page, or send me some pointers so I can do so? I'd love for the
> page to show the right way, maybe an example of rewriting the "bad way"
> into the right way, rather than just giving a stern warning that it's
> bad.
>
> -- Matt
>
> [1]
> https://www.aeolusproject.org/redmine/projects/aeolus/wiki/Coding_Guidelines
>
> [2]
> http://rails-bestpractices.com/posts/27-replace-instance-variable-with-local-variable

In general while I don't completely avoid them, I tend to shy away from
instance variables when I can as they can be used in a somewhat 'global'
manner in rails views.

This doesn't usually pose a problem for primary data being operated on,
for example a provider account which we ensure the user has privileges
on, but for ancillary data, eg data that is loaded to be used in
subsequent lookups or such, the scope of the variable assignment should
be limited to where it is used. Combined w/ places which the views
rendered were directly based on user params without any validation
(almost of all these have been take care of by now), this could have
potentially lead to data being presented to the user which they were not
supposed to have access to.

The guideline might be a bit strongly worded, though it doesn't say
instance variables are prohibited, just should be used only when
necessary (as they often are). This mostly amounts to a scoping issue,
don't use global variables and limit the exposure of data to just where
it is used. We can leverage the named_scopes feature in the activerecord
namespace going forward to filter what is made available, but for the
time being following the guidelines should suffice

  -Mo



More information about the aeolus-devel mailing list