Women in Technology

Hear us Roar

  Rolling with Ruby on Rails, Part 2
Subject:   "Showing recipes in a category" code
Date:   2005-03-04 15:09:19
From:   petercooper
I'm sorry, but the "Showing recipes in a category" code here is not ideal, and certainly not the best/proper way to do what you're trying to do. Consider if you have 2000 recipes in your database, and the category you are trying to view only has one recipe. You've just loaded 2000 recipes into memory to only show 1! What's more, that sort of logic should not be in the view.

Something along the lines of:

@recipes = Category.find(@params['category']?).recipes

will get all the recipes for a specific category, and you won't need any logic in the view to deal with it.

Full Threads Oldest First

Showing messages 1 through 13 of 13.

  • "Showing recipes in a category" code
    2005-03-08 08:19:40  jfister [View]

    I have to agree 100%. It's easy with an application like Rails for beginning programmers to wreak havoc on a server by not being careful with their database queries. Unfortunately, this tutorial does just that in the "Showing recipes in a category" code.

    The working 'list' method of the Recipe controller should look like this:

    def list
    @category = @params['category']

    As pointed out by petercooper, this allows the database server to weed out all records that do not match, rather than loading them all into memory so that the view can filter them.
    • Curt Hibbs photo "Showing recipes in a category" code
      2005-03-08 08:36:56  Curt Hibbs | O'Reilly AuthorO'Reilly Blogger [View]

      Even better, I could have used:

      def list
      @category = @params['category']

      This is an example of dynamic finders, where "find_by_" can be appended with a column name.

      • "Showing recipes in a category" code
        2005-03-10 02:21:34  aeden [View]

        Unfortunately the dynamic find method didn't work on my system. However this does:

        @recipes = Recipe.find_all ["category_id = ?", @category]

        I did have to make some changes to the HTML. First of all the category passed as the argument from the list page should be the category ID and not the name. Second of all you will need to remove the if statement in list.rhtml. I spent 10 minutes trying to figure out why it wasn't showing the filtered recipe list before I realized that this was the problem.
        • Curt Hibbs photo "Showing recipes in a category" code
          2005-03-10 03:37:24  Curt Hibbs | O'Reilly AuthorO'Reilly Blogger [View]

          Oops, you are correct, I should have written:

          def list
          @category_id = @params['category']
          • "Showing recipes in a category" code
            2005-03-11 15:46:16  automat_svet [View]

            I get this error: undefined method `each' for nil:NilClass
            sure I'm doing something wrong in list.rhtml :-(

            any suggestion?
            • "Showing recipes in a category" code
              2005-04-05 12:45:39  co_banzai [View]

              Yeah, I got the same error. I found that I had to make a couple changes to get this working.

              1) On the list.rhtml page, there is a line that reads:

              :category => "#{recipe.category.name}" %>

              This results in it passing the categoty name in the category parameter, which is wrong; it needs to pass the category_id To make things clearer, I also altered the name of the parameter to reflect this change. After doing so, the above line of code was changed to:

              :category_id => "#{recipe.category.id}" %>

              2) I had to update the list method of the recipe controller class in two ways. First, I had to put in a conditional clause so that when the category_id is not defined, it returns all of the data (I'm new to RoR, so there may be a better way to do this). Second, I had to use find_all_by_category_id instead of find_by_category_id. According to the docs, I shouldn't need to do this but I found that without this I got an error (I'm on Rails 0.11). When it was all said and done, my list method looked like so:

              def list
              @category_id = @params['category_id']
              if @category_id
              @recipes = Recipe.find_all_by_category_id(@category_id)
              @recipes = Recipe.find_all

              Hope that helps
              • "Showing recipes in a category" code
                2005-04-22 03:38:42  xeal [View]

                I also think that using the recipe.category.id field is better - after all the recipe.category.name might not be unique.

                And the code simplifies, so you can write the whole RecipeController.list method as:

                def list
                @recipes = Recipe.find_all(@params["category_id"] ? ("category_id = " + @params["category_id"]) : nil)

                The code above passes to Recipe.find_all a category_id, if @params["category_id"] is not empty or nil otherwise.

                Of course, the statement from list.rhtml that renders the category needs to be changed as:

                <td><%= link_to recipe.category.name, :action => "list", :category_id => recipe.category.id %></td>
            • Curt Hibbs photo "Showing recipes in a category" code
              2005-03-11 16:57:37  Curt Hibbs | O'Reilly AuthorO'Reilly Blogger [View]

              First of all, let me say that what I answered above about using @category_id is incorrect, the article is correct as written (I must have been on drugs when I wrote that :-).

              In you case, I'm assuming that you cut'n'paste the code for list.rhtml, so the line on which you are getting the error contains "@recipes.each", which is going to iterate through each member of the collection contained in @recipes.

              The error says the @recipes contains the value "nil" (the Ruby equivalent of Java's null), and that the object "nil" does not have a method named "each".

              That means that something is wrong in the controller where @recipes is being assigned its value. Make sure you list method looks like this:

              def list
              @category = @params['category']
              @recipes = Recipe.find_all

              If it already does, then that means that find_all is failing to find any recipes in the database (or failing to find the database).

              Did you go through part 1 successfully? Does your database contain some recipes?

              You could try starting off the my zip file for your code and use my sql file to initialize your database, and then start working through part 2.
              • "Showing recipes in a category" code
                2005-03-14 07:30:33  automat_svet [View]

                first of all thank you very much for the fast reply and really usefull article.
                I had no problem following your tutorial, the error message came out when I tried to implement the 'find_by' as suggested in the previous comments.
                I'm really new in all this and I'm just tring to learn different ways to do the same job.


  • "Showing recipes in a category" code
    2005-03-04 16:56:48  petercooper [View]

    Not sure why there's a ? in there, as I copy and pasted from something I said on IRC :-) But anyway it should be something like:

    @recipes = Category.find(@params['category']?).recipes

    • "Showing recipes in a category" code
      2005-03-20 14:27:35  garrowsmith [View]

      Can this technique be adapted... say I wanted to list all recipies but sort them alphabetically by category. Is there a neat way to do this?
      • Curt Hibbs photo "Showing recipes in a category" code
        2005-03-20 17:18:44  Curt Hibbs | O'Reilly AuthorO'Reilly Blogger [View]

        Well, one way to do it would be to explicitly specify the SQL and use ORDER BY to do the sorting. Something like this:

        @recipes = Recipe.find_by_sql("SELECT * FROM recipies ORDER BY category_id")
    • "Showing recipes in a category" code
      2005-03-04 16:57:28  petercooper [View]

      Okay, I guess this system puts in random question marks for some reason. Sorry folks :)