incorrect number of arguments

I'm sorry if this should be in the Ruby forum. I decided to put it here
as i'm developing in rails but I'm new to both. I am also wondering if
I'm putting the methods into the right files in rails, therefore I
thought to post in the rails forum.

I am making an appointment booking app and I have a very basic design. I
have created an appointments scaffold with name:string phone:string
email:string numpeople:integer date:date timeslot:string. On the view
for creating a new appointment I have stated that appointment 1 is
9-11am, appointment 2 is 12-2pm, appointment 3 is 3-5pm and appointment
4 is 5 - 7pm. The user is asked to enter 1,2,3 or 4.

When the user clicks on "make appointment" I'm trying to interrupt the
appointments controller (create method) so that I can check if the
date&&timeslot are nil. if that is the case, the system should continue
on to create the appointment, if not then I want to redirect to
somewhere else.

My code is not working and I'm very stuck (as I said I'm very new to
this) Am I going about this in the correct way? I am aware this is not
the best way to go about it, but this is the simplist way I know when I
am not familiar with the language and framework, so please humour my
roundabout methods :slight_smile:

Please can anyone give me some pointers. (The entire appointments
controller is mentioned at the bottom of this post, in case it is
required)

def create # what I have for create so far
    @appointment = Appointment.new(appointment_params)
    @appointments.find(params[:date, :timeslot])

    if @appointments.date.nil? and @appointments.timeslot.nil?
       respond_to do |format|
        if @appointment.save
          format.html { redirect_to @appointment, notice: 'Appointment
was successfully created.' }
          format.json { render :show, status: :created, location:
@appointment }
        elsif
          format.html { render :new }
          format.json { render json: @appointment.errors, status:
:unprocessable_entity }
        end

          redirect_to page_home_path
        end
    end
  end

here are some tips, note that I did not run the code so there might be syntax errors

class AppointmentsController < ApplicationController

before_action :set_appointment, only: [:show, :edit, :update,

:destroy]

def index

load_appointments can be used here

@appointments = Appointment.all

end

def show

end

def new

@appointment = Appointment.new

end

def edit

end

def create

@appointment = Appointment.new(appointment_params)

@appointments has not been loaded, is nil, you will get an error here

you can add a before_action filter and load it there

@appointments.find(params[:date, :timeslot]) # dont load this here since is not always needed

do not use this ‘and’ litely it has a different precedenca than ‘&&’’

you should move this logic to the model, the controller should not be concerned

with persistence logic, look

respond_to do |format|

unless @appointments.has_date_and_timeslot?

some where in your model

def has_date_and_timeslot?

date.present? && timeslot.present?

end

or if you want to use ‘if’ instead of ‘unless’

def missing_date_and_timeslot?

date.nil? && timeslot.nil?

end

if @appointment.save

format.html { redirect_to @appointment, notice: 'Appointment

was successfully created.’ }

format.json { render :show, status: :created, location:

@appointment }

else #elsif # elsif what? you meant ‘else’?

format.html { render :new }

format.json { render json: @appointment.errors, status:

:unprocessable_entity }

end

else # this else was missing I think

here you already called render, you cant redirect, and I think you are missing an else

also you load appointment above only for this to work so move that load here, dont use

a before filter since this will not always be need

load_appointments

redirect_to page_home_path

end

end

end

def update

respond_to do |format|

if @appointment.update(appointment_params)

format.html { redirect_to @appointment, notice: 'Appointment was

successfully updated.’ }

format.json { render :show, status: :ok, location: @appointment

}

else

format.html { render :edit }

format.json { render json: @appointment.errors, status:

:unprocessable_entity }

end

end

end

def destroy

@appointment.destroy

respond_to do |format|

format.html { redirect_to appointments_url, notice: 'Appointment

was successfully destroyed.’ }

format.json { head :no_content }

end

end

private

Use callbacks to share common setup or constraints between

actions.

def set_appointment

@appointment = Appointment.find(params[:id])

end

def load_appointments

# use a pagination game like carrierwave or will_paginate

@appointments = Appointment.all

end

Never trust parameters from the scary internet, only allow the

white list through.

def appointment_params

params.require(:appointment).permit(:name, :phone, :email,

:numpeople, :date, :timeslot)

end

end

Than you so much for your reply and your help! I'm still a little
confused however as I'm still getting an error:

syntax error, unexpected end-of-input, expecting keyword_end

I don't know why it's complaining about end keywords. As far as I can
tell they are ok.

I don't understand what you mean when you say:
"# @appointments has not been loaded, is nil, you will get an error
here
    # you can add a before_action filter and load it there
    @appointments.find(params[:date, :timeslot]) # dont load this here
since is not always needed"

Isn't the @appointments.find(params[:date, :timeslot]) not instanciated
in the index method above?

Do you mean to put a before_action filter in the appointments
controller?

appointments controller:

def create
    @appointment = Appointment.new(appointment_params)

      respond_to do |format|
      unless @appointments.isValid?

        if @appointment.save
          format.html { redirect_to @appointment, notice: 'Appointment
was successfully created.' }
          format.json { render :show, status: :created, location:
@appointment }
        elsif
          format.html { render :new }
          format.json { render json: @appointment.errors, status:
:unprocessable_entity }
        else
          redirect_to root_path
        end
    end
  end

appointments Model:

class Appointment < ActiveRecord::Base

        def isValid?
        date.present? && timeslot.present?
    end
end

There must be a missing end somewhere. Usually when I get this it is
because I have messed up the indenting so it looks ok, but is not.

Colin

Every time you call render or redirect instance disappear from memory, here is how it work: a resquest comes in, the router figures out the controller and action to use and you load/save data, then present it by rendering the view, after this point, the server will wipe out the memory used for that request, so, when you loaded the index, and rendered the index, it was over for the live of that instance, then, the user clicks new, and a new instance is created but with empty attributes, then is used to create the form and passes to the view, after that is wiped from memory. now the form that is created has the data in html and the user insert more data, when the user click submit the server will take those parameters (params) and used it inside Appoinment.new(params) to create a new instance with the attributes that come in the post request of the form, at this point @appointments does not exists, if you try to do anything with its value is nil.

About the end, its could be that a ‘do’ is missing a ‘else’ is incorrect, or a missed an ‘end’ somewhere.

About the end, its could be that a ‘do’ is missing a ‘else’ is incorrect, or a missed an ‘end’ somewhere.

Where is “end 1”? Your indentation is hiding the problem from you as the “unless…” isn’t indented further than the “respond_to do …”

-Rob