Does this Rails 3 Controller method make me look fat?

Question

This is a new application, and I have an index method on a Search controller. This also serves as the home page for the application, and I'm trying to decide if I am headed down the wrong path from a design pattern perspective.

The method is already 35 lines long. Here is what the method does:

3 lines of setting variables to determine what "level" of hierarchical data is being searched.

Another 10 lines to populate some view variables based on whether a subdomain was in the request or not.

A 10 line section to redirect to one of two pages based on:

1) If the user does not have access, and is signed in, and has not yet requested access, tell them "click here to request access to this brand".

2) If the user does not have access, is signed in, and has already requested access, tell them "so and so is reviewing your request".

Another 10 lines to build the dynamic arel.

I can't get it straight in my head how to separate these concerns, or even if they should be separated. I appreciate any help you can offer!

Answer 1

That's a lot of variables being set. Maybe this is a good opportunity for a module of some kind? Perhaps your module can make a lot of these decisions for you, as well as acting as a wrapper for a lot of these variables. Sorry I don't have a more specific answer.

Answer 2

Summarizing what you've said in something codelike (sorry, don't know ruby; consider it pseudocode):

void index() {
  establishHierarchyLevel();
  if (requestIncludedSubdomain())
    fillSubdomainFields();
  else
    fillNonsubdomainFields();
  if (user.isSignedIn() && !user.hasAccess()) {
    if (user.hasRequestedAccess())
      letUserIn();
    else
      adviseUserOfRequestUnderReview();
  }
  buildDynamicArelWhateverThatIs();
}

14 lines instead of 35 (of course, the bodies of the extracted methods will lengthen the overall code, but you can look at this and know what it's doing). Is it worth doing? That really depends on whether it's clearer to you or subsequent programmers. My guess is it's worth doing, that splitting out little code blocks into their own method will make the code easier to maintain.