Code Review: Controller

[gatorjuice]

class ContactsController < ApplicationController
  attr_reader :first_name, :last_name, :email, :phone
  # attr_reader 's belong in the model files, not here in the controller. These aren't doing
  # anything for you here because we aren't ever instantiating a ContactsController 
  # or trying to read attributes of this controller. The model "Contact" has these things.
  def index

    @contacts = Contact.all

    # @all_contacts
    # @contacts.each do |person| 
    #    person = "Name: #{person.first_name} #{person.last_name}, Email: #{person.email}, Phone: #{person.phone}"
    # @all_contacts << person
    # end
    render "index.html.erb"
  end

  def new
    render "new.html.erb"
  end

  def create
    Contact.create(
      first_name: params[:first_name],
      last_name: params[:last_name],
      email: params[:email],
      phone: params[:phone]
    )
    render "create.html.erb"
  end

  def show
    @contact = Contact.find_by(id: params[:id])
    render "show.html.erb"
  end

  def edit
    @contact = Contact.find_by(id: params[:id])
    render "edit.html.erb"
  end

  def update
    @contact = Contact.find_by(id: params[:id])
     Contact.update(@contact,
       first_name: params[:first_name],
       last_name: params[:last_name],
       email: params[:email],
       phone: params[:phone]
     ) 
    render "update.html.erb"
  end

  def destroy
    @contact = Contact.find_by(id: params[:id])
    @contact.destroy
    render "destroy.html.erb"
  end
end

Methods look solid

What you have here looks pretty good, I'd get rid of your commented out section. You want to eliminate code you aren't using and rely on quality git backups for reference and back tracking.

Read my comments in your code about attr_reader

[victorsilent]

Hello! I'm a rails newbie and I have some questions :p 1 - Why don't you use strong paramters? At update and create methods you call each attributes instead use contact_params Reference link: http://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html 2 - Why call .html.erb instead only page name? Like render 'new' by example?

[gatorjuice]

Great questions. We're just being explicit for educational reasons. Rails has a thousand and one shortcuts in it and if you don't learn what's happening explicitly, you may have challenges in the future.

On Sunday, March 27, 2016, victorsilent notifications@github.com wrote:

Hello! I'm a rails newbie and I have some questions :p 1 - Why you don't use strong paramters? At update and create methods you call each attributes instead use contact_params Referenc link: http://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html 2 - Why call .html.erb instead only page name? Like render 'new' by example?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/zeinamakky/contact-app/issues/1#issuecomment-202170818

Sent by Jamie's pocket computer