1 Action w/Get and Post

[Stevenwulf]

Hi,

I'm wondering why rails automatically generates 2 separate actions "new" and "create"

I seem to remember reading something about this on this forum, but I can't seem to find it. Anyway, why not use 1 action "new" and then have different logic depending on if the request is GET or POST. Is there any advantage to splitting it up like this? Is there a disadvantage to having it in 1 action?

One disadvantage I see is when you have select lists. If I want to populate my view with a select list, and if I'm pulling from the db in the controller, then I have to have code pulling the list from the database in both actions, as opposed to one.

Anyway, I'm interested to hear what others are doing.

Thanks,
Steven

[Luke Redpath]

As with all class design, try and keep your class methods small and specific. From this point of view, splitting up the new action (the form) and the create action (the post action) makes sense.

Also, it can help avoid the usability problem of people trying to refresh a page after they have submitted a form (and thus risk resending it) by redirecting to a different page after the post is complete.

If you find duplicate code between the two actions, thats not neccesarily a sign they should be in one action, more that you should consider extracting the common code into a separate, private method.

[Fenrir2]

It does make sense to put them in one action. I think that this is better than the standard scaffold-generated code:

class ProductsController < ApplicationController
	def index
		list
		render :action => 'list'
	end

	def list
		@products = Product.find_all
	end
	
	def show
		load_product
	end
	
	def new
		save_product
	end

	def edit
		load_product
		save_product
	end
	
	def destroy
		load_product
		@product.destroy
		redirect_to :action => 'list'
	end
	
protected
	def save_product
		if @request.post?
			@product.attributes = params[:product]
			if @product.save
				redirect_to :action => 'show', :id => @product
			end
		end
	end
	
	def load_product
		@product = Product.find(params[:id])
	end
end

Your form will look like this:

<% form_for :product, Product.new do |p| %>
	<p>Name:<br />
	<%= p.text_field :name %></p>
	
	<p>Description:<br />
	<%= p.text_area :description %></p>
	
	<p>Save:<br />
	<%= submit_tag 'Save' %></p>
<% end %>

[samsm]

<% form_for :product, Product.new do |p| %>

<% end %>

I had to look up form_for, it's not in the regular api.
It is in the edge documentation:
http://railsmanual.org/module/Action...elper/form_for

Pretty smooth approach in the code above.

[Luke Redpath]

It does make sense to put them in one action. I think that this is better than the standard scaffold-generated code:

class ProductsController < ApplicationController
	def index
		list
		render :action => 'list'
	end

	def list
		@products = Product.find_all
	end
	
	def show
		load_product
	end
	
	def new
		save_product
	end

	def edit
		load_product
		save_product
	end
	
	def destroy
		load_product
		@product.destroy
		redirect_to :action => 'list'
	end
	
protected
	def save_product
		if @request.post?
			@product.attributes = params[:product]
			if @product.save
				redirect_to :action => 'show', :id => @product
			end
		end
	end
	
	def load_product
		@product = Product.find(params[:id])
	end
end

Your form will look like this:

<% form_for :product, Product.new do |p| %>
	<p>Name:<br />
	<%= p.text_field :name %></p>
	
	<p>Description:<br />
	<%= p.text_area :description %></p>
	
	<p>Save:<br />
	<%= submit_tag 'Save' %></p>
<% end %>

Thats not too bad actually. I like the form_for() syntax but I'm not running on edge rails.

[Fenrir2]

It's very easy to switch to edgerails:

1. install subversion
2. open a command line in the directory for which you want to switch to edgerails
3. type: 'rake freeze_edge'

to go back to normal (gem) rails, use 'rake unfreeze_edge'

[Stevenwulf]

Hi Luke and Fenrir2,

Thanks for your feedback. One more question, do either of you think that you should avoid placing call like:

   	<select id="platform_list" name="platform">
   		<%= options_for_select(Platform.find_all.collect {|p| p.name},
 							 Platform.find_all.collect {|p| p.id}) %>
   	</select>

directly in the view? I've always been under the impression that it's more correct--according to strict MVC--to retrieve these values in the action and then pass them to the view. However, in many of the RoR examples I've seen, people are populating lists directly in the view.

Thanks,
Steven

[Fenrir2]

Put this in your controller! And, btw, you are loading all records twice now.

The examples might do it in the view because you only have one code sample then (instead of two).