James Byrne wrote in post #957930:
> Marnen Laibow-Koser wrote in post #957913:
>
>> To me too -- if we're talking *files*. But again: that should never
>> happen. Let's see your views and controllers. I suspect you are trying
>> to cram too much into a single controller, and ending up with lots of
>> actions and views as a result.
>>
>
> I mis-wrote. As you picked up I really meant view directories. But, why
> should it never happen for view directories?
Why should *what* never happen? That you should have lots of view
directories? I specifically didn't say that that should never happen.
>
> I am writing an application that handles international freight bookings,
> movements and customs formalities including edi cusdec transmissions.
> That is never ever going to fit into a handful of controllers.
Of course it won't. I didn't mean to suggest that it would. In fact, I
was suggesting more controllers, not fewer.
If you don't like the number of controllers, group them in namespaces.
> In fact,
> but for foreign exchange, most controllers possess only the basic index,
> show, new, create, and destroy actions. Many do not have all of these.
>
> A sample controller, one of the more complex ones, is given below:
>
> class CustomsShipmentParsChecksController < ApplicationController
>
> skip_before_filter :authenticated
> skip_before_filter :authorised
>
> # POST /collection
> # POST /collection.xml
> def create
>
> respond_to do |format|
> if @manifest = CaCustomsManifest.find_by_ccdn(
> params[:ca_customs_manifest][:ccdn].strip)
>
> @shipment = @manifest.ca_customs_shipment
> @entry = @shipment.ca_customs_entry
>
> if
> @manifest.ca_customs_shipment.ca_customs_entry.is_across_accepted
> flash[:info] = accepted_message
> # render simply uses the template specified by the :action key
> # without calling the method.
> format.html { render :action => "show" }
> format.xml { render :xml => @shipment,
> :status => :accepted,
> :location =>
> :customs_shipment_pars_check }
> else
> flash[:notice] = not_accepted_message
> format.html { render :action => "show", :id => @shipment.id }
> format.xml { render :xml => @manifest,
> :status => :found,
> :location =>
> :customs_shipment_pars_check }
> end
> else
> new # build a dummy record complex to allow display of messages
> manifest.ccdn = params[:ca_customs_manifest][:ccdn].strip
> flash[:notice] = not_found_message
> flash[:warn] = contact_message
> format.html { render :action => "new" }
> format.xml { render :xml => @manifest.errors,
> :status => :not_found }
> end
> end
> end
Too long. Most of that should be refactored into model methods.
Remember, "skinny controller, fat model". A controller method over 5-10
lines is a sure sign that you need to do more refactoring.
Best,
--
Marnen Laibow-Koser
http://www.marnen.org
marnen@marnen.org
--
Posted via http://www.ruby-forum.com/.
--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To post to this group, send email to rubyonrails-talk@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
No comments:
Post a Comment