Newby to SQL, creating simple Hotel Database/Housekeeping Database

Thanks, that gives me a lot of good information that I can change, some if it I caught myself already, but thank you!

The only thing I'm concerned about getting is how do I make rooms change from clean to dirty when they are checking into?
So right now I have people in rooms that are set as '0' which is clean, but when the rooms are occupied this needs to be automatically changed to '1' for Dirty, I'm having a lot of problems with this.

You could use a Trigger for that, or make it part of the SProc that handles "Checking In".

How will Checking In work? You could have a Bit Field on the RESERVATION table that says "Checked In"? If so the act of changing the IsCheckedIn flag to 1 could trigger setting the corresponding ROOM record's ROOM_STATUS to "dirty"

If there are other means of setting IsCheckedIn then it might be best to have a Trigger on RESERVATION that sets ROOM_STATUS to Dirty on any INSERTS / UPDATES to RESERVATION where IsCheckedIn changes from 0 to 1.

I don't suppose you want to bother about it just now, but what about:

If IsCheckedIn is accidentally set and is then "reversed". The ROOM_STATUS will have been set to Dirty, should that be reversed?

What about if I stay for 3 nights - when I Check In the room status becomes Dirty, what will set that on the next two days so my room gets cleaned?

Missed your earlier post with SProcs. Some thoughts:

CREATE PROCEDURE prc_Add_Guest
@GUEST_ID int
, @GUEST_LNAME varchar(15)
, @GUEST_FNAME varchar(15)
, @GUEST_PHONE varchar(10)
, @GUEST_LICENSEPLATE varchar(10)
, @GUEST_LICENSEPLATE_STATE char(6)
, @ErrorNo int = 0 OUTPUT
AS
/*
 * prc_Add_Guest adds a person to the guest table
 *
 * Returns:
 *
 *	nothing
 *
 * HISTORY:
 *
 * 08-Dec-2016 KLDR  Started
 */
SET NOCOUNT ON
SET XACT_ABORT ON
SET ARITHABORT ON

-- Any validation code here, if errors found set @ErrNo and GOTO prc_Add_Guest_EXIT

BEGIN TRANSACTION

INSERT INTO GUEST 
(
     GUEST_ID, GUEST_LNAME, GUEST_FNAME, GUEST_PHONE, GUEST_LICENSEPLATE
    , GUEST_LICENSEPLATE_STATE
)
VALUES 
(
    @GUEST_ID, @GUEST_LNAME, @GUEST_FNAME, @GUEST_PHONE, @GUEST_LICENSEPLATE
    , @GUEST_LICENSEPLATE_STATE
)
SELECT @ErrorNo = @@ERROR
IF @ErrorNo <> 0 GOTO prc_Add_Guest_ABORT

-- ... more Insert / Updates here if required ...

prc_Add_Guest_ABORT:

IF @ErrorNo = 0
BEGIN
    COMMIT
END
ELSE
BEGIN
    ROLLBACK
END

prc_Add_Guest_EXIT:

RETURN @ErrorNo

Move the comment higher up - so that it is "at the top" when you review the SProc for any reason.

ALWAYS include a column list in your INSERT statement. Otherwise if you add a column to the table your INSERT will fail (because the VALUES list will be one-short). Also, without a column list your INSERT statement is dependent on the ORDER of the columns, its a really bad idea to rely on that anywhere because it might easily change in future.

We use SET XACT_ABORT ON in all our Sprocs, it causes any errors in child SProcs etc. to propagate up to this one and force this SProc to also fail, otherwise it is possible for some errors to go unnoticed. I'd be interested to know if other folk here have strong views on using this (or not using it!)

The other two SET statements just provide a consistent platform. You almost certainly do not want NOCOUNT OFF (silly name, its a double-negative)

It may feel a bit weird, but I suggest you START a continuation line with the delimiting comma, and not END the line with it. There's a thread on here somewhere about the discussion we had and generally speaking it was preferred. A few swings and roundabouts, but I converted relatively recently and I can say that I have found that the Pros FAR outweigh the occasional Con.

I've also put a TRANSACTION around your INSERT. Not actually needed as the INSERT will either succeed or fail, but if you were to add a second Insert / Update (e.g. to set the ROOM_STATUS during Check In) then you would have the possibility of one working and the other failing, and you don't want just part of the code to alter the database, you want either everything to succeed (COMMIT), or nothing (ROLLBACK)

I've added an ErrorNo value to trap any errors. This is returned as a Parameter so you can include an (optional) parameter for that in the EXEC if the "caller" needs to know success/fail, and it is also returned. So the caller can do:

DECLARE @MyErrorParam int
...
EXEC prc_Add_Guest @GUEST_ID=1234, ..., @ErrorNo = @MyErrorParam OUTPUT
or
EXEC @MyErrorParam = prc_Add_Guest @GUEST_ID=1234, ...,

We have LOTS of STUFF in our standard template for SProcs. I don't want to weigh you down right now :slight_smile: but I would advise that before you build anything particularly significant that you ask what people here / elsewhere use, and form a comprehensive template. We have changed ours, through experience, on a couple of occasions over the last decade and going back and refactoring ALL the existing code to use the new template was a complete PITA ... no Profit from that exercise, just more resilience, which is very hard to put a cash / time value on ...

Same suggestions for prc_Add_Reservation

(I've kept your Sproc names, but I suggest you consider changing them to Noun-Verb as I mentioned earlier)

prc_Delete_Reservation is passing several parameters which are not used in the body of the Sproc. I would get rid of those extra parameters (or use them to validate that the row being deleted matches them all)

You might want to add a test that @RESERVATION_ID exists and return an error number / validation warning message if it does not.

You are passing @GUEST_ID to prc_Add_Guest - how will the operator know a suitable ID number (which is not already in use)? Same for @RESERVATION_ID in prc_Add_Reservation

One option would be to use the IDENTITY parameter on those columns in the CREATE TABLE statement. That will automatically assign next-available-ID-number.

If you do that I have a recommendation. The IDENTITY function takes a "Seed" value, i.e. the first ID number to allocate. Rather than starting all tables at 1 / the same number, start them at different numbers. e.g. GUEST_ID from 1000, RESERVATION_ID from 2000 and so on. Your test data will only have a few rows, so the IDs will never overlap. If you accidentally JOIN the wrong ID there will be no matching data, so you will find out quickly that there is a logic error in your code.

Again, I still think the business data model needs fleshed out more. Don't rush into the implementation details before you have the core data elements well enough refined, or you'll just have to do a ton of re-design and re-coding later.

Let's start with "reservation". By your design, it appears to me that a guest must have a reservation. Is that really true? That is, if someone walks into the lobby who didn't make a prior reservation but wants to book a room, will he/she be refused? At most places, typically not.

I think's that why I'd be more likely to want room "bookings" and "reservations" as separate things. Some bookings were never reservations, and some reservations never become bookings (the person never showed up to claim the room).

So right now I have people in rooms that are set as '0' which is clean, but when the rooms are occupied this needs to be automatically changed to '1' for Dirty,

I'd incline more to an minimum elapsed time room was occupied for "dirty". If a room's just been cleaned, it's not dirty the second sometimes steps into it. For example, if I just got the room at 7:50AM, I might be annoyed if I had to fend off a room service request at 8:02AM -- it's your hotel, don't you know I just got here??

I thought that, but then considered that a walk-in customer could be checked in by creating a "reservation" for them (could have a flag for Booking / Reservation perhaps) ... dunno if that is a good idea though!