#259 Decent Exposure

I've been exercising a similar practice manually for a while now and I can confirm it really makes controllers more concise and readable in 99% of the cases. Good to know it has been wrapped up as a Gem now.

It definitely does clean things up quite a bit. It kind of makes me think of the actions as belonging to a ghost town though with nothing in them.

Soon you won't have to write any code and rails will do it for you lol.

Are there any speed or memory benefits to using methods rather than instance variables other than the more robust action caching?

I got inmediately excited and was loving every second of this video. I can recognise that DecentExposure is a pattern in itself, and a wonderful one, it should make it into Rails core!

Much cleaner + leaner than sharing instance variables!

This should be Rails default behavior!

I'm with @miguelsan and @Tilo. My spontaneous thought too was that this should be in Rails core.

I wonder how seamlessly you could get this to work with declarative_authorization...

and +1 to @Nate Bird's question.

@Nate, I am guessing any other speed/memory differences would be minimal. After the methods are defined (happens when class is loaded) it should be quite fast. But I haven't done any benchmarks.

@MissingHandle, I don't know about decl_auth, but I hope to get support for this into CanCan 2.0. You can keep track of the issue here.

https://github.com/ryanb/cancan/issues/#issue/317

After reading the somewhat cool review of The Rails 3 Way at RubyInside, where the author complains that a "curious technique called Decent Exposure is pushed as a way to never use instance variables in your views", I knew I had to try it.

And right on cue you do a great episode on DecentExposure. Works great and although I didn't A/B it against standard Rails controller methods, it seems quite snappy. Thanks for your superb work.

I definitely like the approach.
Because "@" should be used for addressing people ;).
@miguelsan,@tio: Agree with you, one should not even have to call expose for RESTful resources. I would rather prefer to have to tell the controller when *not* to expose or when the model name and controller name do not match. We could have nested resources exposed automatically too, by inspecting the routes.
@Ryan: I am confused about the default behavior of the expose(:comment) in the nested resource example. I understood, that you left it as the default because there was no id param. Would that not have worked too? I mean by delegating to the scope of :comments, which was article.comments, would that not have yielded article.comments.find(param[:id]) ?
Cheers
Robert

I've tried a similar thing (without decent_exposure) and I find it a bit confusing sometimes. For example, when you have an edit action where you can edit any article, and a show action where only certain articles can be accessed.

Then you end up calling things like "public_article" in the view (similar to Ryan's example in the cast). That means there's some coupling between the view and the responsibility to decide whether an article can be accessed in a certain action or not.

So far I'm not used to it and prefer the approach where the view receives an article and doesn't care about where it came from, but I'll give it another try sometime.

The first regression test I ran after changing an existing controller over to using this failed on:

    assert_not_nil assigns(:users)

Since instance variables aren't being set, clearly assigns() will always return nil.

examining 'users.size' in the test returns 0.

Visually examining the response.body, I can see that it is correct. Any ideas for a replacement assert?

To answer my own question, here's one possibility:

    assert @controller.users.size > 0

Not sure if I like it. Actually, pretty sure I don't.

@moabite, we tend to use integration tests for that sort of thing at Hashrocket. Asserting against the content of an instance variable in your tests means you're coupling with a specific part of an implementation. Interrogating the UI for the presence of an expected element (likely the name of a user or users in this case), allows you to only assert things you care about.

@rbates thanks for the coverage! For more background on what motivated me to start using this pattern and subsequently encapsulate it as a gem, check out this post: http://blog.voxdolo.me/a-diatribe-on-maintaining-state.html

For declarative_authorization:

I just tried out the pattern (instead of the gem) to get a feel for it, as you showed in the first half. And I used the load_method option on filter_access_to. It cleaned the controller up beautifully!

Also - if you want your controller specs to pass, I think you need to make those private methods before_filters on your controllers. But, not really understanding action caching, would the before filters negate the benefit of it?

@Stephen Caudill, Thank you for the response.

To make sure I understand, what you are suggesting is that in the functional test to only check the result in general terms, e.g. 'assert_response :success' or 'assert_redirected_to some_path' etc depending on the expected result, and in an integration test look through the response.body for detailed content?

Making controllers downright anorexic.

I'm wondering like @moabite how you adapt your functional tests. I typically test the state of controller instance variables using the assigns method.

@moabite, @tomrossi - I've stopped testing things via assigns entirely and check for the specific things I expect to see in the UI via integration tests (by interrogating request.body using a higher level encapsulation like RSpec/Capybara or Cucumber). For the most part I only use controller tests for unit testing 3rd party API integration or things I can't test via Selenium (like testing around Flash), and don't do any functional testing of my controllers.

@stephen, how do you keep those integration tests from becoming brittle?

@MissingHandle it's a delicate balance, but I find myself doing things like this:

Given the following users:
  | first name | Chucky |
  | last name | Myers |
When I am viewing the user index
Then I should see "Chucky Myers" in the "Users" section

Where 'in the "Users" section' is a way of scoping to a portion of the page (you can see our scoping steps in this gist: https://gist.github.com/894947)... This helps us only couple to the things that we need to see on the page in order to make effective assertions.

Again, it's a delicate balance that's more art (peppered with science) than anything.

About view testing with rspec and decent_exposure:
http://blog.angelbob.com/posts/379-RSpec-Haml-and-decent-exposure-are-incompatible-That-d-be-odd----published-rails

Just mock it:
view.should_receive(:entry).at_least(:once).and_return(my_entry)

this should be a default option on the next release of rails? no?

Not to be too much of a theorist, but doesn't this blur the model & controller responsibilities. With decent_exposure you are replacing the model object with a controller helper method. I guess if I look at it as simply a wrapper for the model object it's a wash between the two. Just my initial reaction was we are bending MVC rules some. Anybody else get that feeling?

As long as we're talking theory & coupling...

From the perspective of the view, it has to 'know' a priori how the controller is providing data, either instance variables or expose'd methods etc. I've been changing some controllers over to using respond_to/respond_with ala episode 224 (http://railscasts.com/episodes/224-controllers-in-rails-3). The only stumbling block for me is that the view template still needs to know how the controller is providing data, even though the controller specified 'respond_with widgets' (or 'respond_with @widgets'). Non-templated requests, e.g. /widgets/index.xml, are picking up the correct data, but I don't see a clean way of determining in the view template (say index.html.haml) what the controller said to respond_with. The real pain point here is when I'm rendering partials. At the moment I'm having to call the partial with a :locals hash.

Of course, I could be way out in left field here. If I am, please leave some breadcrumbs so I can get back home.

@Stephen Caudill, thank you! that makes so much sense. We've been doing too much specific assertion where if we change the name of an id of say, a link, things break.

Also, your blog post made me think that this application of the Single Responsibility Principle makes a lot of sense to integrate into cucumber step definitions down to maybe the link/id/css class level? Instead of calling a link or id directly, you call a method that returns the link/id. That way if you change a link or id, there's only one place to change it...that seems like it might cure the brittleness i'm hurting from and keep the features exact.

@Bill Christian - to add to @moabite's comments, I think the controller is supposed to interact and know about models. Certainly, the view shouldn't make model calls, but the controller has to - it's the middle man between the two, no? Or maybe I miss your point?

I think you keep model/controller responsibilities separated by keeping the controller focused on scoping models and referring to as few model attributes as possible.

@MissingHandle: I never thought the view shouldn't make model calls as it is the necessary data to display. Which models to use is the controller's responsibility.

Outside of the DRY-ing up of the controller code, I just am not realizing the advantage of using a controller method call rather than an instance variable. After all, the method is returning the same object. However, I am looking at this from one simple (BUT AWESOME) demonstration. I'll definitely do some experimenting.

Very nice... I would have stopped before installing the gem however. Your example of just adding a couple of methods makes things clear and concise but adding the gem unnecessarily hides the simple logic away making it hard to see exactly what is going on without delving into the gem code/docs.

It looks like I might be the only one, but for some reason I don't like this at all. I've always liked instance variables in the view because the @ makes it very clear where the data is coming from -- the controller. Everything else is either local or a helper method. This makes that much harder to see at a glance.

The other issue is what happens if on an article page you decide to have a "related articles" list and do a loop like:

<% for article in related_articles %>

What happens with 'article' in this case after the loop completes? Seems messy. Yes, I could change 'article' to 'a' or something, but the chance for collision bothers me. Even having to remember that in this case article is not from the controller, but a local variable bother me...

+2 cents.

@Phillip Hallstrom: in the scope of the iterator, article will be passed in and you won't see an article you've exposed via the controller. This is probably fine though, because you're not likely to need to deal with the exposed article inside that iterator.

As to differentiating, the same could be said of methods in models.. "How do I know if it's a method or a variable?!" I can only say that it gets easier with time and is made even easier by tools integrating things like (exuberant) Ctags.

The simple fact of the matter is that instance variables are meant to be referenced internally by an instance of a class. Outside objects (like an instance of ActionView) having direct access to another object's internal state is an excellent way to increase coupling and doesn't abide by OO best practices like "programming to an interface".

This approach was guided largely by my attempt to adhere to OO design advice offered by tomes written by people much smarter than myself, such as the GoF's Design Patterns book.

@Ryan : how do you convert to decent exposure something like episodes controller on the railscasts.com app?

e.g

https://github.com/ryanb/railscasts/blob/master/app/controllers/episodes_controller.rb

Found out that if you put expose(:current_user){current_user} in combination with devise, this one little line is almost enough to access all submodules through it like a global var with meaning. In later controllers (assuming you have to log in as user) just using 'current_user.offers' etc

I'm so sorry but have to tell you this too: take my previous comment, then in the mentioned offers controller add

expose(:offers){current_user.offers}
expose(:offer)

and voila...you have a sort of..caching nesting how much I can see

The one of drawbacks of using instance variables is that they are nil by default, so your code could silently work incorrectly. decent_exposure help to catch unsused variables. thanks!

I like the idea of an explicit interface between controller and view, but I'm not quite sold on this particular solution just yet. Especially since the cancan gem already fulfills most of this, and also handles authorization.

Looking at https://github.com/ryanb/cancan/issues/317 it seems this combined solution is already in the works :)