Ruby on Rails Thursday, July 1, 2010

Robert Walker wrote:
> Pale Horse wrote:
>> My controller code can be seen here:
>>
>> http://pastie.org/1026480
>
> Oh, where to begin. Unless I'm completely daft, I see only one single
> line in your new method that belongs in a "new" action.

I can't argue with that but this is not my site. Frankly, I don't have
the *time* to make those kinds of amendments across the site, so in the
light of consistency, I will not make one here. Perhaps when I obtain
more time, I will return to this project and tighten this code. I am,
equally, not happy with this method. Despite this, it *certainly* works.

> This one:
> @project = Project.new
>
> Compare your method with what a new method should look like in a
> controller:

Thank you, I'm well aware what is generated by the scaffolding.
Digression...

> Example:
> # GET /presentations/new
> # GET /presentations/new.xml
> def new
> @presentation = Presentation.new
>
> respond_to do |format|
> format.html # new.html.erb
> format.xml { render :xml => @presentation }
> end
> end
>
> The new method of a controller serves one purpose (following the single
> responsibility principal). That purpose is to GET the blank form. In
> other words the blank form is considered to be a resource that request
> using a GET method having a resource identifier similar to:
>
> http://example.com/presentations/new
>
> Take note that the @presentation object is used only for context to draw
> the form. When the form is submitted that instance of @presentation gets
> thrown away. A new instance is then initialized at the beginning of the
> "create" method as you can see in the following example:
>
> Example:
> # POST /presentations
> # POST /presentations.xml
> def create
> @presentation = Presentation.new(params[:presentation])
>
> respond_to do |format|
> if @presentation.save
> flash[:notice] = 'Presentation was successfully created.'
> format.html { redirect_to(@presentation) }
> format.xml { render :xml => @presentation, :status => :created,
> :location => @presentation }
> else
> format.html { render :action => "new" }
> format.xml { render :xml => @presentation.errors, :status =>
> :unprocessable_entity }
> end
> end
> end
>
> Your "new" method seems to be attempting to take on both
> responsibilities. I know there is such a thing as a post-back pattern,
> but the sorts of problems you're having now are why that pattern is
> strongly discouraged in modern web application design.

I'm well aware that this is not the correct method to invoke this
action.

> Besides all that, if all you're attempting to do is set a default state
> for your boolean fields when creating new records, then set a default
> state in your migration (... :default => false).

Again, I'm well aware that default values can be set in migrations.

Now, I realised that a lib file was being included. One that the
developer prior to myself had (for some reason) put in and had written
himself for a generator. On initialising this, it sets the value of
'display' to true regardless of the state in which it is.

Conclusively, check your inclusions if the site you're working on was
not created by yourself.
--
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