Bad Programming practices in rails
1. Not using associations properly
Say, we have association of College has many students, having foreign key college_id in student table and the relation as below
class College < ApplicationRecord has_many :students end class Student < ApplicationRecord belongs_to :college end
Bad Practice:
Getting all students in a college
students = Student.where(college_id : 3)
Good Practice:
students = college.students
2. Hashes: Using fetch when key may not be present
Let's take an Hash called params below. Using dig is more efficient than fetch.
params = { "booking" => "flights", "trip_id" => "06574637", "refundable" => true }
Difference between fetch and dig
Fetch throws exception when key does not exist.
params.fetch("booking")
=> "flights"
params.fetch("something")
KeyError: key not found: "something"
from (pry):22:in `fetch'
Dig throws nil even if key does not exist
params.dig("booking")
=> "flights"
params.dig("something")
=> nil
params.fetch("something", nil)
=> nil
So, I prefer using dig rather than fetch for avoiding exceptions in the code.
3. Using nil? always, rather than present?
nil? returns false for empty array [] and empty string "" . nil? only checks whether the variable or object is nil or not.
[21] pry(main)> nil.nil?
=> true
[22] pry(main)> "".nil?
=> false
[23] pry(main)> [].nil?
=> false
[24] pry(main)> {}.nil?
=> false
[25] pry(main)> nil.present?
=> false
[26] pry(main)> {}.present?
=> false
[27] pry(main)> [].present?
=> false
[28] pry(main)> nil.blank?
=> true
[29] pry(main)> "".blank?
=> true
[30] pry(main)> [].blank?
=> true
[31] pry(main)> {}.blank?
=> true
blank? is kinda opposite to present? and equally efficient.
4. Not using strong parameters
ActionController::Parameters inheriting from object class provides methods like params, require and permit. It's always recommended to use strong parameters.
Allows you to choose which attributes should be whitelisted for mass updating and thus prevent accidentally exposing that which shouldn't be exposed. Provides two methods for this purpose: require and permit. The former is used to mark parameters as required. The latter is used to set the parameter as permitted and limit which attributes should be allowed for mass updating.
person_params = params[:person] Person.new(person_params)
Will take all attributes for mass updation. Whereas, using require and permit allows name and age attributes to be whitelisted.
Person.new(person_params) def person_params params.require(:person).permit(:name, :age) end
5. Using each when returning an array while looping
each loops into each element in an array, whereas map returns array of elements returned in each iteration.
array = [1, 2, 3, 4, 5, 6, 8, 9, 10]
result = []
def select_even_nos
array.each do |e|
result << e if e.even?
end
result
end
def select_even_nos
result = array.map { |e| e if e.even? }
end
Next Post
Are you sure using “students = college.students” is better than using “students = Student.where(college_id : 3)” ?
Because “students = Student.where(college_id : 3)” executes only one query to fetch all the students.
Where as “students = college.students ” executes two queries to fetch the same result. One for constructing the college object and another for getting the students.
Hi Deepak,
Thank you very much for asking this question. I appreciate it.
The college.students is a “has_many” relation from application record. students = college.students does the same as students = Student.where(college_id : 3) from the backend.
As asked above, in both cases, the query made is only one query. Because college object is already present as we call it from the model.
It is always preferable using “college.students” because Ruby on Rails runs on convention over configuration. It deals with real world entities, rather than just coding.
Hmmm, if I recieve college_id as a parameter, wouldn’t it be better to use Student.where(college_id : param_college_id)? So I don’t need to query college because I don’t need to verify the existence of college, there’s a constraint in the database for that.
But I agreed that Student.where(college_id :college.id) would be really weird.
college.students is also better because it hides the internal implementation of the relationship. If at a future point the app is rewritten to make students a HMT or HABTM the expression “college.students” continues to work fine.
Thank you Steve for making us understand better 🙂
You can use @college= college.includes (:students) and in view call college.students in only one query.
Thank you Steve and LR-dev for making the post better 🙂
Helpful 🙂
Thank you very much Bishnu Basyal. Please do share this article to your friends and colleagues 🙂
array.map do { |e| e if e.even? }
this doesn’t work ( ‘do’ invalid, and if returns nil or even number ), so we have to rewrite it
array.map{ |e| e if e.even? }.flatten
but shortest way to do it:
array.select(&:even?)
Hi Malleus,
Yes, as you said above it doesn’t work. It was a typo. Thank you very much for rectifying it. I have edited the post.
Thank you
Also that the example given does not give the same result. map gives nil when if condition not satisfied. map is a transformative function so there will not be any change in the no of elements out vs in. The right Function to use is select
2.2.5 :043 > select_even_nos array
=> [2, 4, 6, 8, 10]
2.2.5 :044 > select_even_nos_map array
=> [nil, 2, nil, 4, nil, 6, 8, nil, 10]
You would have to append a compact after the map block. You type less but I have my worries when dealing with large arrays since it’s creating an extra object for that.
But you going to find a lot of examples of code creating an unnecessary object for the sake of looks, which isn’t in itself a bad thing, just… not optimized enough for me.
I am assured, what is it to me at all does not approach. Who else, what can prompt?
Rexuiz FPS Game
One thing I want to say is the fact that before getting more computer memory, consider the machine in to which it would be installed. In the event the machine is definitely running Windows XP, for instance, a memory limit is 3.25GB. Using over this would merely constitute a new waste. Make certain that one’s mother board can handle your upgrade amount, as well. Interesting blog post.
Rexuiz FPS
While Rails is certainly MVC-centric, nothing prevents you from creating your own types of classes and adding appropriate directories to hold the code for those classes. When you have additional functionality, think about which methods group together and find good names for the classes that hold those methods. Using a comprehensive framework like Rails is not an excuse to let good object oriented design best practices go by the wayside.