You should really read basics about variable types in ruby. All your $... vars are globals. It's not threadsafe and really hugly.
You should really learn to give readable names to your vars (ex: params[:elements][:province]). Why $elementspro = params[:elementprovince] #... :conditions => "province=" + $elementspro instead of :conditions => "province=" + params[:elementprovince] ??????. your code will be more readable and threadsafe Why :conditions => "province=" + $element instead of :conditions => ['province = :elementprovince', params] ????? your code will be more readable AND SAFE!!! I agree with Michael, you clearly come from php. Ruby is not php. First, there's threads like in any other correct language and it means you have to handle with. Second, in rails, every good practice is often (always?!) simpler to use than bad practice. Every rails tutorial use good SQL practice, why not you? Even your html is ugly. - Instead of millions of checkboxes, use multiple lists - Don't write your javascript in your html page - Don't use divs (or anything else) out of body - Don't declare body anywhere else than in your layouts - Don't use logic in view (page = params[:page]) => will_paginate handle nil params[:page] for you - Use cool syntax like: page = params[:page] || 1 - Don't use table, unless for tabular data presentation (table is a table, not a visual tool) - Don't use style propertie in html, use css in separated css file(s) NEVER USE GLOBAL VARS! it's really rare when you can justify of their using. I think you're clearly not ready to use mvc and oop, go read manuals. You just proved, another time, that most of php coders suck. On 15 mar, 02:46, Michael Graff <[email protected]> wrote: > With code like this: > > def advanced_search > $elementspro = params[:elementprovince] > $elementstype = params[:elementstype] > $elementsequipment = params[:elementsequipment] > $elementdown = params[:elementdown] > $elementstatus = params[:elementstatus] > $elementapproval = params[:elementapproval] > $elementteam = params[:elementteam] > $elementstaffname = params[:elementstaffname] > $frm_view_setup = params[:frm_view_setup] > @tblpss_description_records = TblpssDescriptionRecord.paginate :page > => params[:page], :conditions=>"province='" + $elementspro + "'" + "or > type_of_job='" + $elementstype + "'" + > "or equipment_part='" + $elementsequipment + "'" + "or sitedown='" + > $elementdown + "'" + "or job_status='" + $elementstatus + "'" + > "or job_status='" + $elementapproval + "'" + "or team_on_job='" + > $elementteam + "'" + "or RecorderName='" + $elementstaffname + "'", > :order => 'jobNo ASC', :per_page => $per_page > end > > you might as well post your database password in public. This is > dangerous code. It allows SQL injection. > > It's pretty clear you come from a PHP world. I suggest you read a few > books on Ruby programming, and google a bit for "rails sql injection." > Your code is a security nightmare. --~--~---------~--~----~------------~-------~--~----~ 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 [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
