all 9 comments

[–]simon_jester_jr 2 points3 points  (0 children)

Flowers and Chocolates are always a good idea.

[–]zaphnod 0 points1 point  (6 children)

I mean, first off, you're defining global functions, which means your @customer variable isn't being saved. Wrap the whole thing in a class ReservationSystem ... end, and then instantiate one of them with system = ReservationSystem.new. :-)

ActiveRecord#find_or_create_by will create the customer in the DB if it doesn't find a match on the name entered, assuming you have everything set up correctly. I'm assuming you're using SQLite or something?

Your reservation model is using name as its key, which isn't ideal. Usually you'd use the implicitly created Customer#id field. And the reservation.name == self won't work because self doesn't point to your customer. You could do reservation.name == @customer.name, but the better solution would be to do @customer.reservations, again depending on how you have your model set up. ActiveRecord relations are designed to let you do what you want without messing with finders. To create your reservations, you'd use @customer.reservations.create(result), and you wouldn't prompt for a name (it's owned by the customer, right?) and AR would automatically set the Reservation's customerId to @customer.id.

Lots of questions about your schema. Would be helpful to copy what you're using for DB setup, models, etc into a pastebin or somesuch.

[–]zaj89[S] 1 point2 points  (0 children)

I’ll see if I can post most of my project into a pastebin a little later this evening when I get home !

[–]zaj89[S] 1 point2 points  (4 children)

https://pastebin.com/GYmVjhxU this is my controller, most of my global functions

https://pastebin.com/CGkwxMLM this is my schema

https://pastebin.com/ZpxVFqq4 this is my customer model, with a few functions and a many-to-many function of get restaurants

after reading the advice of you and another poster i really hit it hard last night trying to solve it and this is what i came up with, it all works and is demonstrating full CRUD, it may not be the prettiest, but it works, thank you all! and i used a global variable as theres only ever one user at a time to be using the app so the context was appropriate i feel, let me know what you think! and now i can start to add some new features to it since the base is working, like reviews, popular dishes, and pick by cuisine is what im thinking for my new features to add to it

[–]zaphnod 1 point2 points  (0 children)

Looking good!

A couple of pointers:

Your list_reservations is redundant, ActiveRecord has_many relationships allow you to just do $customer.reservations - the reservations method is automatically created when you declare the has_many relationship. Ditto for get_restaurants - just use $customer.restaurants and you're good to go. ActiveRecord is one of the nicer parts of the Ruby ecosystem. :-) Note that you can also do something like $customer.reservations.order('date DESC') to order from most to least recent.

Another small point: when declaring a foreign key in a migration/schema, it's clearer to use e.g. t.references :customer than t.integer 'customer_id'

Great work man, you're kicking ass. :-)

[–]zaphnod 1 point2 points  (2 children)

Also, while acceptable if your project never grows, using global variables is virtually never the proper solution. Right now, you only have one global for your customer, but as you add more features, you'll start needed history and context that travels through your business logic. Get used to making classes/objects out of everything, because that is the way Ruby is designed to work and will promote better coding. :-)

Also, more comments. Whoever is grading this will like comments. Make sure to comment your intentions, rather than what you're doing to achieve those intentions. The code says what, the comments say why. Cheers!

[–]zaj89[S] 0 points1 point  (1 child)

Wow thank you a bunch! You’ve been very helpful! I’m just scratching the surface of active record so not yet fully sure of all its capabilities so I’ll continue to read it’s docs. As for the global variable solution I used, I know it’s usually a no-no for global variables but currently I’m stumped and couldn’t figure out another solution, I just wanted something that could get my project “ready to ship” so that I atleast have a working version to hand in friday, but since that is in a commit, now I can push it a bit and see if I can expand on this. It’s been a very tough project for our second week and I’m doing it alone where everyone else in my class had a partner(I was the odd man out, from a 15 people class)

[–]zaphnod 0 points1 point  (0 children)

Here's a pattern I use:

class ReservationSystem
  def self.start
    system = self.new

  def login
    ... your code, access instance vars with @customer, etc ...


You create a class to contain your methods & state, declare a static method (in this case start) to do any setup and to instantiate the global instance of your class, then at the end of the file, you call that static method.

This lets you have the benefit of instance variables and encapsulation, with minimal overhead.

[–]xmplr 0 points1 point  (0 children)

ActiveRecord though probably the most common library for database interaction in the ruby world, might not be the best way to start with. This depends on the goal of your current lesson. If it aims for you to be able to understand SQL, then you might be better off with writing the SQL yourself.

Still, you might want to use it as a inspiration.

It uses some basic methods to create, find, update (safe), and delete stuff from your database while mapping it to your objects variables.

A stubbed reservation class might look like this

``` Class Reservation

## attributes basically match your table columns attr_accessor :id, :people_count, :date, :restaurant_id, :customer_id

some methods for the actual SQL interaction

def self.find_by_id(id) ## basically a factory method. ## Select all data from SQL for the reservation id and assign them to the attributes of an instance of the reservation class, see above end

def self.find_by_customer_id(customer_id) ## do SQL ## another factory method ## you are looking for a foreign key, therefore this one might return an array of reservation objects ## a customer might have multiple reservations end

def save ## do some SQL. update the column with the id, set the attributes (see above) ## delete is basically the same end

def restaurant ## a helper method, a getter for the associated restaurant object Restaurant.find_by_id(restaurant_id) end

def restaurant=(restaurant_object) ## another helper, setter for the associated restaurant object restaurant_id = restaurant_object.id end end ```

You might want to add some more methods, this is just a basic stub and rough immitation of what active record does. However, this is basically a model class, which does the DB interaction for you. Now when you want to create a reservation, initialize a new reservation object. Prompt your user for its data and assign the values accordingly. Then call the "safe" method for your object to persist data in the database.

When you need associated information, like the restaurant, you ask for the restaurant's name and call something like "Restaurant.find_by_name" to get the respective object. Then you use the reservation objects setter to add the restaurants primary key value to the foreign key field.

Reservations might also become deleted when the delete method on a Customer or Restaurant object is called, to which the reservations are related. This is called referential integrity.