- favour clarity even if it means more lines of code
- write code for people first
- unix single reponsibility principle
- minimize non presentation logic in templates
- alignments of code
- data model design, less than ideal naming in model, names matter
- reduce roundtrips to the database
- semantic css
- it is ok to have multiple css files
- templates
- helpers or not helpers
- data model
- naming - current_user
- authorization - admin - over engineer
- authorization methods
- repeating parts in layout in a partial
- side bar menu in a partial
- application helper better as a partial
@authenticated.houses.first.id
- make use of activerecord validations
error checking when saving, consider this example
if @item.save
redirect_to items_path # goto items list when save successfully
else
render :new # show the new form again when saving failed
end
- less specific css
- list of products, possible candidate for a partial
- form helpers are ruby code can be used in haml or erb
- products/show.html.erb
consider refactorings:
def view_by?(user)
user.id == meal.id
end
def edit_by?(user
user.admin? || user.id == meal.id
end
if @company_authenticated.present? && @company_authenticated.id == @product.company_id
elsif @authenticated.nil?
elsif (@product.questions.map(&:id) & @authenticated.answers.map(&:question_id)).length == 0
orders/index.html.haml
- partials
activereocord adds attribute methods for you so you don't the following
def email
self.email
end
consider naming @current_user instead of @auth
def authentication
@auth = User.find(session[:user_id]) unless session[:user_id].nil?
end
meal.rb
def view_by?(user)
user.id == meal.id
end
def edit_by?(user
user.admin? || user.id == meal.id
end
orders controller edit
def edit
@order = Order.find(params[:id])
@meals = []
@meals << @order.meal
end
semantic css
%th.second Meal
%th.first Location
%th.fifth Host
%th.third Time
%th.fourth Price
%th.fifth Message
heroku private key in repo?
full size image used everywhere
def owned_categories
Category.where :owner_id => self.id
end
due to less than ideal naming
would be nice and should be able to do
@user.categories
to get all categories belonging to this user and
@user.followings
@user.followers
to get the respective followings and followers
another approach is to use self joins with a following model
string interpolation is unnecessary here:
def search
@pictures = Picture.where("name ilike ?", "%#{params[:query]}%")
@users = User.where("username ilike ?", "%#{params[:query]}%")
end
@pictures = Picture.where("name ilike ?", params[:query])
- good use of collection partials
things like this can be improved
if @picture.id.in? @authenticated.favourites.map &:picture_id
- data model is important because they form a foundation, and hard to change later
naming matters
User ---< Wish >--- Bar
User has many wishes Bar has many wishes
Wish belongs to User Whish belongs to Bar
Wishlist ---< Wish
Wishlist has many wishes Wish belongs to Wishlist
the following is an example of doing too much:
def authentication
return unless (session[:user_id] || session[:company_id]) # Check for IDs to authenticate.
@user_authenticated = User.find_by_id session[:user_id] unless session[:user_id].nil?
@company_authenticated = Company.find_by_id session[:company_id] unless session[:company_id].nil?
# If we didn't log anyone in, clear out the sessions so we don't try again.
if @user_authenticated.nil? && @company_authenticated.nil?
session[:user_id] = nil
session[:company_id] = nil
end
end
better naming
event ---< participation >--- unit
roles, does it need to be a model?
def is_admin?
roles.include?(Role.where(:name => 'admin').first)
end
be careful not to over engineer, you may not need it
start with admin attribute in user
@user.admin?
move on to role attribute in user
def admin?
role == 'admin'
end
move on to roles attribute in user as an array
def admin?
roles.include? 'admin'
end
move on to role as a separate model & table
has_many :roles
def admin?
roles.include? Role::ADMIN
end