Hi. I am often checking the same parameters (like param[:page] and param[:per_page]) in models over and over again (are those in a specific range ... if not use defaults): page = !options[:page].blank? && options[:page] =~ /^[0-9]+$/ && options[:page].to_i > 0 ? options[:page].to_i : 1 per_page = !options[:per_page].blank? && options[:per_page] =~ /^[0-9]+ $/ && options[:per_page].to_i > 0 && options[:per_page].to_i <= 100 ? options[:per_page].to_i : 10 How can I make this more DRY and easier to read? -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
turkan wrote in post #964861:> Hi. > > I am often checking the same parameters (like param[:page] and > param[:per_page]) in models over and over again (are those in a > specific range ... if not use defaults): > page = !options[:page].blank? && options[:page] =~ /^[0-9]+$/ && > options[:page].to_i > 0 ? options[:page].to_i : 1 > per_page = !options[:per_page].blank? && options[:per_page] =~ /^[0-9]+ > $/ && options[:per_page].to_i > 0 && options[:per_page].to_i <= 100 ? > options[:per_page].to_i : 10 > > How can I make this more DRY and easier to read?Well, first of all, the code you provided is a classic example of obfuscated Ruby. You can make it more readable by using if instead of ?: . Now, as far as the repetition of the conditions themselves, just do the same thing you''d do for any other repeated code: extract it into a method of its own. Best, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On 29 November 2010 20:30, turkan <kai.schlamp-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:> Hi. > > I am often checking the same parameters (like param[:page] and > param[:per_page]) in models over and over again (are those in a > specific range ... if not use defaults): > page = !options[:page].blank? && options[:page] =~ /^[0-9]+$/ && > options[:page].to_i > 0 ? options[:page].to_i : 1 > per_page = !options[:per_page].blank? && options[:per_page] =~ /^[0-9]+ > $/ && options[:per_page].to_i > 0 && options[:per_page].to_i <= 100 ? > options[:per_page].to_i : 10 > > How can I make this more DRY and easier to read?You can make it easier to read by using simpler constructs such as if. It will take up a few more lines of source code but will not have any measurable effect on execution time, and will be much easier for those that come after you to understand. You can DRY it by putting it in a method. I also have to wonder why a _model_ is worrying about page number and per_page. Colin -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
> You can DRY it by putting it in a method.In what method should I put it that is visible in all models? In an extra lib class?> > I also have to wonder why a _model_ is worrying about page number and per_page.I pass it to a model method that does some Sphinx search. What would be a better way? The search expression is so long, so I thought it would be good to keep it out of my controller (everybody says "keep your controller thin"). -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
turkan wrote in post #964900:>> You can DRY it by putting it in a method. > > In what method should I put it that is visible in all models? In an > extra lib class?Probably.> >> >> I also have to wonder why a _model_ is worrying about page number and per_page. > > I pass it to a model method that does some Sphinx search. What would > be a better way? The search expression is so long, so I thought it > would be good to keep it out of my controller (everybody says "keep > your controller thin").Then you probably want a separate model to talk to Sphinx, then call Sphinx.search(params[:criteria]) in your controller. Or maybe you want a utility class to do this. It really depends on the structure of your app. Best, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
Thanks for the answers. One last general question. Where do you validate your parameters fetched by "params" that should be passed to models? Do you check them right away in the controller and give them cleaned up to the model, or do you just provide them as they are (maybe just put them in single variables) and let the model check them? -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
Please quote when replying. Kai Schlamp wrote in post #965115:> Thanks for the answers. > > One last general question. Where do you validate your parameters > fetched by "params" that should be passed to models? Do you check them > right away in the controller and give them cleaned up to the model, or > do you just provide them as they are (maybe just put them in single > variables) and let the model check them?It''s usually the model''s job to validate. That''s what the validates_* methods are for. Validation in the controller will lead to repetitive code scattered all over the application. Best, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On Nov 29, 3:30 pm, turkan <kai.schl...-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:> Hi. > > I am often checking the same parameters (like param[:page] and > param[:per_page]) in models over and over again (are those in a > specific range ... if not use defaults): > page = !options[:page].blank? && options[:page] =~ /^[0-9]+$/ && > options[:page].to_i > 0 ? options[:page].to_i : 1 > per_page = !options[:per_page].blank? && options[:per_page] =~ /^[0-9]+ > $/ && options[:per_page].to_i > 0 && options[:per_page].to_i <= 100 ? > options[:per_page].to_i : 10 > > How can I make this more DRY and easier to read?For a start, make use of the fact that Ruby is not Java. For instance: ''''.to_i # => 0 nil.to_i # => 0 ''blargh''.to_i # => 0 ''123foo''.to_i # => 123 That last one is arguably weird, but reasonable. Secondly, for range-ish cases you might want to use some of the Array functions. Your first case becomes: [1, options[:page].to_i].max the second (this is not as clear): [100, [1, options[:per_page].to_i].max].min or, if you don''t mind scribbling on the options hash: options[:per_page] = 10 unless (1..100).include? (options[:per_page].to_i) --Matt Jones -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org 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.
> Validation in the controller will lead to repetitive code scattered all > over the application.Ok, then one last last question ;-) ... just want to make sure that I do it the Rails way. When you let the user pass several paramters regarding how the instances of your model are presented (order, page, and so on), where do you check those? Do make such (sometimes more complicated) queries in the controller (and check those params there) or define a special search method in the model (and check also those parameters in the model)? -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On Nov 30, 2010, at 3:27 PM, Kai Schlamp wrote:>> Validation in the controller will lead to repetitive code scattered >> all >> over the application. > > Ok, then one last last question ;-) ... just want to make sure that I > do it the Rails way. > > When you let the user pass several paramters regarding how the > instances of your model are presented (order, page, and so on), where > do you check those? > Do make such (sometimes more complicated) queries in the controller > (and check those params there) or define a special search method in > the model (and check also those parameters in the model)?What are you checking these parameters for, exactly? Everything has to pass through the model to the database, the model is responsible for sanitizing any SQL that is needed. Further, the model can enforce any business logic requirements you define in your validates* filters. If you have a search facility in your page, you probably want to run that through either a separate search method in your controller, again, passing back whatever the user has entered to the model for eventual processing; or maybe you want a separate Search model and controller to interface with Sphinx or Solr or whatever you might be using under the hood. A separate model also allows you to search across multiple models without having to repeat a search controller method in each of those models'' controllers. Walter -- 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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.