Unexpected nil object

[dan06]

I've put together a simple app, when run displays the error below. Anyone know why the instance variable (@news_form) is nil? See below for my controller and view code. Thanks.

You have a nil object when you didn't expect it!
You might have expected an instance of ActiveRecord::Base.
The error occurred while evaluating nil.update_attributes

Controller:

class NewsController < ApplicationController 
  def add_news
    @news_form = News.new
  end
  
  def save_news
    @news_form.update_attributes(params[:news_form])
    if @news_form.save
      redirect_to :action => "news"
    else
      render :action => "add_news"
    end
  end 
end

View:

<%= error_messages_for :news_form %>
<h1>Add News</h1>
<%- form_for @news_form, :url => { :action => :save_news } do |f| -%>
<fieldset>
<legend>Enter News Details</legend>
<%= f.label :title, "Title:" %>
<%= f.text_field :title %>
<br />
<%= f.label :description, "Description:" %>
<%= f.text_area :description %>
</p>
<p><%= f.submit "Add" %></p>
<%- end -%>
<%= link_to "View News", :action => "news" %>
</fieldset>

[7stud]

The add_news action never executed?

[ReggieB]

Items are not preserved from one view to the next. You have to define them in the controller for each view. To keep things DRY, if you need to create the same objects in a number of views it is worth stripping them out to a separate private method that is then called from the various views that require them.

Also update_attributes calls save so calling an additional save method is superfluous (see the api docs).

And standard Rails convention is to use a simpler name for an instance of a class. So an instance of News is called news and therefore your instance variable would be @news rather than @news_form. Your view code would need to be altered to use this line:

  <&#37;- form_for @news, :url => { :action => :save_news } do |f| -%>

You could even use

  <%= form('news') %>

To generate a simple form (see api docs again)

The params object you then need to grab is params[:news] rather than params[:news_form]

Personally, I'd check that the params exist before trying to update the attributes.

So try this version of your controller:

class NewsController < ApplicationController 
  def add_news
    generate_creation_objects
  end
  
  def save_news
    generate_creation_objects
    if params[:news] and @news.update_attributes(params[:news])
      redirect_to :action => "news"
    else
      render :action => "add_news"
    end
  end 

  private
  def generate_creation_objects
    @news = News.new
  end
end

[ReggieB]

It would also be worth considering changing your view/action names:

'add_news' to 'new'

and

'save_news' to 'create'

as these are in line with the usual Rails conventions and would unlock things like restful routing.

[dan06]

Items are not preserved from one view to the next. You have to define them in the controller for each view.

By 'items' you mean variables, including instance variables i.e. @news_form, right? So, if I understand - in order to access an instance variable from within the same class I need a getter method?

And standard Rails convention is to use a simpler name for an instance of a class. So an instance of News is called news and therefore your instance variable would be @news rather than @news_form.

Thanks for the heads-up on Rails conventions; I'm new to ruby/rails, just started a few days ago so there's a lot for me to learn/get used to.

Personally, I'd check that the params exist before trying to update the attributes.

I used validate_presence_of in the model to ensure there were values before attempting to update; is that validation better done in the controller?

I ended up changing the controller (and the views accordingly) to the following. If you see any other rails convention inconsistencies, let me know.

class NewsController < ApplicationController
  def news
    @news = News.find :all, :select => "title, description", :order => "id DESC"
  end
   
  def new
    @news = News.new
    if params[:news] and @news.update_attributes!(params[:news])
      redirect_to :action => "news"
    else
      render :action => "new"
    end
  end 
end

[ReggieB]

By 'items' you mean variables, including instance variables i.e. @news_form, right? So, if I understand - in order to access an instance variable from within the same class I need a getter method?

No. If anything, I should have said 'Objects' rather than 'Items'. Within a class you can pass information between methods via instance variables. So this will work:

class Thing
  def initialize
    @stuff = 'Hello World!'
  end

  def output_stuff
    @stuff
  end
end
thing = Thing.new
thing.output_stuff     ---->  'Hello World!'

The point I was making concerned passing information between web pages. The best way of thinking about this I think, is that when your browser requests a page, Rails creates an instance of the Controller class, then calls the method name that matches the request. When the browser then requests the next page, Rails creates another instance of the Controller class and calls the method that matches the new request. So you cannot directly pass information between the method that matches one view, with the method that matches the next view because the two were created with different instances of the Controller.

This is a thought model of how it works:

#Request page /one/one
one_controller_instance_1 = OneController.new
render one_controller_instance_1.one

#Request page /one/two
one_controller_instance_2 = OneController.new
render one_controller_instance_2.two

one_controller_instance_1.one cannot directly talk to one_controller_instance_2.two, because one_controller_instance_1 is a different obejct to one_controller_instance_2. Therefore the new instance has to be able to create its instance variables on its own.

Thanks for the heads-up on Rails conventions; I'm new to ruby/rails, just started a few days ago so there's a lot for me to learn/get used to.

It would be worth working through some of the tutorial and/or basic books so that you understand the basic conventions before you start breaking them. In Rails you have the full Ruby tool kit available to you - so you can do anything that Ruby will allow. However, Rails works most easily within the conventions. You are free to break the conventions. There are often good reasons to break the conventions, but that should always be a concious decision. It can only be a conscious decision if you understand the conventions before you break them. Therefore, I strongly recommend you get used to working with Rails during the learning process, and that means keeping with the conventions .... to start with

[QUOTE=dan06;4347094]
I used validate_presence_of in the model to ensure there were values before attempting to update; is that validation better done in the controller? [\QUOTE]

validate_presence_of will check content being passed to an individual method/attribute. It won't check to see if there is anything there to be passed to the update_attributes= method. That is, what happens when params[:news] doesn't exist.

I ended up changing the controller (and the views accordingly) to the following. If you see any other rails convention inconsistencies, let me know.

As I said above - I recommend you work through the basic tutorials until you get an understanding of the Rails conventions first. You obviously have previous coding experience and it may seem annoying that you need to go back to basics, but I am sure the annoyance is worth it. Convention over configuration is key to many of the simplification and automation that makes Rails so quick and easy to use. If you are to get the most out of Rails, you need to understand the conventions - even if (as I suggested above) you find yourself breaking them from time to time.

[dan06]

The point I was making concerned passing information between web pages... when your browser requests a page, Rails creates an instance of the Controller class, then calls the method name that matches the request. When the browser then requests the next page, Rails creates another instance of the Controller class and calls the method that matches the new request. So you cannot directly pass information between the method that matches one view, with the method that matches the next view because the two were created with different instances of the Controller.

Thanks for the explanation; I was a bit confused as to why an instance variable was not available between views.

It would be worth working through some of the tutorial and/or basic books so that you understand the basic conventions before you start breaking them... Therefore, I strongly recommend you get used to working with Rails during the learning process, and that means keeping with the conventions .... to start with

...You obviously have previous coding experience and it may seem annoying that you need to go back to basics, but I am sure the annoyance is worth it. Convention over configuration is key to many of the simplification and automation that makes Rails so quick and easy to use. If you are to get the most out of Rails, you need to understand the conventions - even if (as I suggested above) you find yourself breaking them from time to time.

I am using a book to help me with learning ruby/rails; I went through the introductory chapters on rails and then jumped right in... without knowing (and realizing that there are so many) rails conventions. Since your post I've gone through the later more detailed chapters that explained the conventions. And you're absolutely right, it is a bit annoying to learn these conventions - but even at this early stage I can see the benefits of using rails (and the conventions). If you have any books/tutorials I should look into please let me know.

Just in case anyone is interested below is a revised version of my practice code, which I believe (fingers crossed) follows all rails conventions:

[CODE]
class NewsController < ApplicationController
before_filter :lookup_news, :except => [:index, :new, :create]

def index
@news = News.find :all, :select => "id, title, description", rder => "id DESC"
end

def new
@news = News.new
end

def create
@news = News.new(params[:news])
if @news.save
redirect_to :action => "news"
else
render :action => "new"
end
end

def edit
end

def update
end

def destroy
@news.destroy
redirect_to :action => "index"
end

protected
def lookup_news
@news = News.find(params[:id])
end
end
[CODE]