feedback.md

• 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

refactorings

• templates
• helpers or not helpers
• data model
• naming - current_user
• authorization - admin - over engineer
• authorization methods

airbnbguide

possible refactoring considerations

• repeating parts in layout in a partial
• side bar menu in a partial
• application helper better as a partial

warning bells

@authenticated.houses.first.id 

general suggestions

• 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

focusme

• 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

NBFood

refactorings

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

warning bells

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?

pictures app

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

barhopper

• 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

cubbylife

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