UML class diagram for a system.

[JamesBwoii]

Hi, I need to make a UML class diagram for a hotel billing system then implement it in Java. I'm quite new to UML so was just wondering a few things.

The scenario is:

• 3 different types of customer - corporate, group and individual
• Corporate customers are local companies employees with a strong customer relationship.
• Group customers are generally families who share a single room.
• Individual customers use a single room.
• To automate billing the hotel must be able to store customer’s name, address, telephone numbers, the dates and number of days stayed in the hotel.
• System must automatically generate a bill for each customer.
• Flat rate for every customer of £100 per person per night.
• Corporate customers are given a 20% discount, and 50% discount on weekends.
• Group bookings are an extra 10% for every person sharing the room.
• Individual customers are charged standard rate.

I need to perform an Object Oriented design and represent using a class diagram. I need to identify all objects in the system and their methods and attributes.

I need to use polymorphism to calculate the bill.

What are the classes that I need? At the moment I can think of;

• Hotel
• Customer with corporate, group and individual as subclasses.
• Rooms where hotel has a finite number of rooms.

Thanks for the help.

 

[Nono713]

How about a class that identifies a booking (so it would hold the customer and the room(s) booked), as well as a HotelBillingSystem (or whatever) class which keeps a log of active bookings and also computes how much to bill each customer based on what they booked. This is where the business logic (the different special cases on pricing) would go, where each booking is translated into a bill for the customer to pay.

How did the Lisp assignment go, by the way?!

 

[JamesBwoii]

Oh yeah, long time no speak!

It went pretty well, we don't know our exact marks but I got at least a 2:1. Not sure if you have them in New Zealand but it's the second best after a 1st. Where I lost marks was mainly my report/write up as he said I didn't explain some stuff well enough or give enough complex examples. I disagree with him as he gave me an example I should have tested, but I had in fact tested one basically the same as that.

And that makes sense, I'll have a go and making a class diagram then, thanks! :D

 

[JamesBwoii]

I've had a go and was wondering if you could take a quick look just to see if there's any issues or changes I need to make.

Thanks!

View attachment 3937

 

[Nono713]

Hi again,

there are a few good ideas in this design but also some severe problems. Here are some comments:

1. I think you have some of your arrows and labels backwards, for instance did you really mean for a Hotel to contain only one Room, and for a Room to be associated with arbitrarily many Hotels? ;) Similarly, did you mean for HotelBooking to derive from Room? Seems like you meant composition here. Make sure to check your arrows according to the UML spec.

2. The Hotel class as-is seems a little bit redundant, containing only a name and an address and apparently unused. Since your assignment doesn't seem to mention a hotel anyway (only its billing system) I would personally scrap it.

3. The Customer hierarchy has a big problem: the hierarchy is not useful! Given a Customer instance you have no way of knowing how to bill it without finding out which type of customer (individual/corporate/group) it actually is, which defeats the purpose of using polymorphism. This is a common mistake. Now you have two main ways out of this situation, which are:

- give the Customer class a "calculateBill" method which simply takes a HotelBooking instance and returns how much the customer should be billed for that booking, this gets the job done but is somewhat brittle and introduces some weird dependency chains (the HotelBooking depends on the Customer which depends on the HotelBooking and so on). It also means your Customer class is "active": it "does stuff", contains logic. I would not recommend this approach except in the simplest of scenarios, really.

OR

- factor out all of the information necessary for determining how much to bill out to the Customer class, but doesn't actually carry out the billing calculation: it is "passive". This requires some thought: what determines the bill? I see three things: flat rate (which applies to everyone) discounts (corporate/group bookings, note some discounts depend on the booking time) and finally extras, in the case of group bookings. Let's consider all three of them:

* Flat rate: you made the correct choice in moving the flat rate out to the hotel billing system, it doesn't depend on any customer and is fixed by the hotel. For instance, suppose the flat rate changes one day: it doesn't make any sense to go back and change it for every single customer.

* Discounts: this is likely the trickiest one, because some discounts apply only on weekends for instance. The simplest way to do this is a Discount class, with a single method getDiscount, which takes anything the discount depends on (in this case, a date). This seems pretty general. Now you can create specific derived classes that inherit from Discount, for instance a CorporateDiscount class with a getDiscount method which returns 20% on weekdays and 50% on weekends.

* Extras: this could be as simple as an ExtraCharge (or whatever) class which returns how much extra to charge the bill. You can then create specialized versions via e.g. a GroupExtra class which could work as e.g. new GroupExtra(5 [guests]) represents an extra 50% charge on the bill.

Then you can see that your Customer class eventually boils down to just a name/address/phone + an optional discount and extra charge, and this might not seem like a huge difference, but it actually is: now the billing system can consume any Customer instance without needing to know if it's a corporate customer or a group customer or whatnot: everything it needs is right there in the form of discounts and extras. And what happens to the CorporateCustomer/GroupCustomer/etc. classes, then? Well, simple: they are simply Customers, but their constructor defines the appropriate discounts and extras (in fact, it would probably be more sensible to use the factory pattern here). Done. In fact, if you look closely the only thing that is kind of problematic is the "companyName" in the CorporateCustomer class, but it actually isn't a problem: corporate customers have names, too, and the hotel billing system doesn't care about the company name at all. Also, with this scheme you can actually have arbitrarily many discounts and extra charges, and they stack either multiplicatively or additively. You don't appear to need this for your assignment, but it is worth considering in that the system is extensible to support more complex billing schemes.

4. Finally, the HotelBillingSystem is a little barebones. Reflect on how the system would be used: it should probably take in bookings (how the bookings are stored is not really the billing system's concern) and return a bill for the booking. In that respect, perhaps the calculateBill() method should take a HotelBooking parameter, and calculates the bill for that booking. Last minor thing, the bill should probably not be expressed as an integer (especially since you have percentages). An appropriate type is "Currency", probably. So you'd have e.g. "calculateBill(): Currency".

This is quite a long post, so please if you have any questions make sure to quote the relevant part and I'd be happy to go over it in detail. Some of the suggestions I made here probably go above and beyond what would be expected from the assignment (read: overengineered) but you may wish to understand them to perfect your own billing system design.

 

[JamesBwoii]

Thanks for such a detailed reply! I've gone through and tried to make the changes you've suggested but am getting a bit confused on bits. Here's what I've got at the moment.

View attachment 3948

Firstly,

* Discounts: this is likely the trickiest one, because some discounts apply only on weekends for instance. The simplest way to do this is a Discount class, with a single method getDiscount, which takes anything the discount depends on (in this case, a date). This seems pretty general. Now you can create specific derived classes that inherit from Discount, for instance a CorporateDiscount class with a getDiscount method which returns 20% on weekdays and 50% on weekends.

I've made the discount class and the corporate discount inherited class but I'm not sure what they should be connected to? Is it the HotelBillingSystem class?

Secondly I'm not sure if I've done what you meant with the customer class? I've got it so the 3 different types of customer all use the customer class as an interface but at the moment they are all identical. I don't have a method for storing a discount that should be applied to that customer as I'm not sure if that is correct. Should the extra charge, seeing as it's only applied to group bookings also go in the 3 different customer classes?

Thanks :D

 

[Nono713]

Hi, that looks quite a bit better! To answer your questions:

I've made the discount class and the corporate discount inherited class but I'm not sure what they should be connected to? Is it the HotelBillingSystem class?

The discounts are attached to the Customer interface: each customer instance can have up to one discount associated with it: corporate customers will have a CorporateDiscount attached to them, for instance (others will just have none: null, I guess). Technically the most general implementation would accept arbitrarily many discounts, and combine them multiplicatively or additively somehow, but since you only have one type of discount this is unnecessary here.

Similarly for the extra charges, each customer instance can have up to one extra charge, here there is only one type of extra charge for the group bookings.

This ties into your second question...

Secondly I'm not sure if I've done what you meant with the customer class? I've got it so the 3 different types of customer all use the customer class as an interface but at the moment they are all identical. I don't have a method for storing a discount that should be applied to that customer as I'm not sure if that is correct. Should the extra charge, seeing as it's only applied to group bookings also go in the 3 different customer classes?

The key here is to reason abstractly: if you consider a group customer, a corporate customer, and an individual customer, there is no fundamental difference between them besides the fact that some types of customers get discounts or charges others don't. So the three classes are, indeed, functionally identical. The only difference here would be in the constructor of each: the CorporateCustomer's constructor would take a name, address and phone as usual, but would then automatically attach the appropriate discount. Similarly for the GroupCustomer's constructor, which would attach the extra charge for groups. That way you are still using polymorphism, but you do not need to reason on the type of the customer: all the information needed to bill a customer is already in the interface (or, to be more specific, you have already reasoned on the customer's type: when you created it; in essence, instead of saying "I have a customer here, let me see what type of customer he is so I can bill him", you're saying "I am creating a corporate customer, and advertising that this customer has a 20% discount, 50% on weekends"). Note the flexibility in that you can effectively "customize" customers (no pun intended) by giving them custom discounts and charges without changing the underlying logic. This is what is referred to as data-driven design. If, later on, you find that different types of customers should actually have more structural differences, the design may be reconsidered, but this seems unlikely for a Customer class which almost by construction shouldn't really be "doing" anything but should instead be a simple data container, a record of sorts. In fact this means that the Customer interface should probably be an abstract base class rather than an interface as the customer subclasses are identical in behaviour (and just differ in the data they contain) but this is not a huge deal.

Basically, this changes the perspective from "a corporate customer is a type of customer" to "customers have attributes such as discounts and extra charges which affect the way they are billed, and a corporate customer happens to be a customer with a corporate discount". They don't really teach you this in most CS courses but composition (which is what you are using here: you are composing discounts and charges with the Customer interface to extend behaviour) is usually much more powerful than inheritance, simply because composition behaves better with respect to dependency chains and separation of concerns. For instance, I mentioned in my previous post that you could in principle keep using inheritance by sticking a "calculateBill" on the customer interface. After all, the only difference (from the perspective of the billing system, anyway) between the customers is the way they are billed. But is it really up to the customer classes to calculate their own bill? Not really, because they don't have all the information. For instance, some rooms could be king size and lead to a higher bill, and so on, and at that point you may as well pass the HotelBooking and the flat rate to the customer class, which means the HotelBooking becomes hopelessly coupled to the Customer hierarchy, and they may as well be merged. With composition, on the other hand, the billing system simply takes in the HotelBooking, looks at the room involved, the length of the booking, the customer (and hence the discounts he is entitled to) and computes the bill from that. Much more straightforward, and doesn't lead to the problematic coupling, since the customer is just one additional factor in the billing system, not a critical component of it, and the HotelBillingSystem class actually does what it's supposed to be doing: calculating billls, rather than delegating that somewhere else.

In some ways composition is far more fundamental than inheritance, but it's hard to see that if you just got introduced to the merits of inheritance the instant you walked through the lecture hall door (like I did, by the way). It's just a tool in your toolbox, but is sadly too often regarded as the ultimate tool that solves every problem.

Please do ask for clarification as needed, this is quite tricky stuff and took me a while to grasp the utility of.

 

[JamesBwoii]

I've read through what you've said a few times and think I've got my head around it and see what you're saying. I'm just struggling to relate that to my UML diagram. View attachment 3950

I've made some changes.

Firstly, I've added a numberOfGuests field to the customer and by extension the different types of customer. This is obviously important for group and potentially corporate bookings as these could contain more than 1 person, however it's also in the individualCustomer class where it is essentially useless as that will always remain at 1. Is this a problem or is it OK as it it is? I was thinking that it might be better if it was placed in the hotelBooking class.

I've also added the extraCharge and groupExtra classes as well as discount and corporateDiscount.

Thanks! :)

 

[Nono713]

I've read through what you've said a few times and think I've got my head around it and see what you're saying. I'm just struggling to relate that to my UML diagram.

I've made some changes.

Firstly, I've added a numberOfGuests field to the customer and by extension the different types of customer. This is obviously important for group and potentially corporate bookings as these could contain more than 1 person, however it's also in the individualCustomer class where it is essentially useless as that will always remain at 1. Is this a problem or is it OK as it it is? I was thinking that it might be better if it was placed in the hotelBooking class.

I've also added the extraCharge and groupExtra classes as well as discount and corporateDiscount.

Thanks! :)

Looking good, still a couple questions:

1. what is the difference between calculateExtra() and getCharge() in the extra charge classes?

2. it seems like a mistake to put the numberOfGuests inside the customer class - or at least, it seems redundant, since you are already going to use that information in the groupExtra class, correct? (in its constructor) Both are admissible, though since you already have a discount class you may as well have an extra charge one as well.

3. one thing that I forgot to mention before, what exactly does the "getRoomsAvailable" method do?

4. I am not sure what your conventions are, but I think class names are usually capitalized (pascal case) LikeThis.

Other than that, it seems good, the HotelBooking class makes sense (a Room + a Customer + a range of days for which the room is booked) and the HotelBillingSystem does exactly what it's meant to do and no more than that.

Remember that the getDiscount() method takes a date on which the discount is to be calculated for, since corporate discounts are not the same on week days and weekends.

 

[JamesBwoii]

1. what is the difference between calculateExtra() and getCharge() in the extra charge classes?

In my head I did this so that calculateExtra() would hold the logic needed the calculate the extra charge and then getCharge() would be an accessor to get the charge.

2. it seems like a mistake to put the numberOfGuests inside the customer class - or at least, it seems redundant, since you are already going to use that information in the groupExtra class, correct? (in its constructor) Both are admissible, though since you already have a discount class you may as well have an extra charge one as well.

I'm not completely sure what you mean here. It's obviously important to know the number of guests that a customer has but I don't know what class it belongs in.

3. one thing that I forgot to mention before, what exactly does the "getRoomsAvailable" method do?

For this one I was unsure of if the hotel should be able to take infinite bookings and therefore have infinite rooms. The specification says that we can assume the hotel has a maximum of 20 individual, 20 corporate and 10 group customers meaning I could in theory just give the hotel 50 rooms and that would be enough if every single customer decided to stay at the hotel at once. With regards to the getDiscount() needing to take a date does it take the dateFrom and dateTo from the hotelBooking class?

:D

 

[Nono713]

I'm not completely sure what you mean here. It's obviously important to know the number of guests that a customer has but I don't know what class it belongs in.

Think about the groupExtra class: how are you going to create it? It is probably going to have a constructor.. which takes the number of guests that the group customer has (and derive the appropriate charge from that: if it is passed 3 guests, then it will return a 30% charge in its getCharge() method for instance). That new groupExtra is then attached as an extra charge to the group customer's Customer instance.

To summarize, for a group customer you would do:

myGroupCustomer = new GroupCustomer(name, phone, address, numberOfGuests);

That constructor would itself do something like this:

this.name = name;
this.phone = phone;
this.address = address;
this.extraCharge = new GroupExtra(numberOfGuests);
// can now discard numberOfGuests: billing system uses the extraCharge

(here "this" represents the GroupCustomer instance inside the constructor)

In contrast, for a corporate customer it would look more like:

myCorporateCustomer = new CorporateCustomer(name, phone, address);

And its constructor is (something like):

this.name = name;
this.phone = phone;
this.address = address;
this.discount = new CorporateDiscount();

Does that make sense? The idea, as always, is that "extra charges" are a more abstract, more general, more usable concept than "number of guests" (which are already by definition coupled to the GroupCustomer class, so that's what happens, they appear in its constructor but are then transparently converted to the appropriate extra charge for the billing system to use). Similarly a time-dependent "discount" is more general than "corporate discount" (which obviously only applies to corporate customers), so if you wanted you could create a new type of discount and/or a new type of customer with a special 80% discount and the billing system would remain unchanged. No need to add any more logic, you can just leverage the existing discount/charge implementation to extend your billing system (and if you want to, you can add a completely new extra attribute alongside the discounts/charges, though a good system tries to minimize complexity).

For this one I was unsure of if the hotel should be able to take infinite bookings and therefore have infinite rooms. The specification says that we can assume the hotel has a maximum of 20 individual, 20 corporate and 10 group customers meaning I could in theory just give the hotel 50 rooms and that would be enough if every single customer decided to stay at the hotel at once.

With regards to the getDiscount() needing to take a date does it take the dateFrom and dateTo from the hotelBooking class?

Hm, strange, I wouldn't expect these kinds of details for a billing system implementation. Is it just flavour text in the assignment or are you actually meant to implement (or at least design) the booking system as well as the billing system? I was under the impression only the billing system needed to be designed (which has no business allocating rooms or even knowing how many rooms are available). In any case, stick to the assignment specification.

For your other question, it takes any date and computes the discount on that date, that's the beauty of it. But, yes, for the purposes of calculating the bill as defined in the assignment the billing system would pass it each day the room is booked (so between dateFrom and dateTo) and it would calculate the appropriate discount for that particular day. (I assume the total bill is the sum of the bills for each individual day the room is booked).

 

[JamesBwoii]

Ok, I see where you're coming from. I'm not sure if I fully understood what you're saying, but I've removed the numberOfGuests from the Customer class and put it in the ExtraCharge class and the GroupExtra class?

About the specification, it's slightly vague but looking at it again, I don't think it matters about the room booking. If that is the case, is the Room class redundant?

I've attached the relevant part of the spec just in case you wanted a quick look.

:D

View attachment 3962

 

[Nono713]

Ok, I see where you're coming from. I'm not sure if I fully understood what you're saying, but I've removed the numberOfGuests from the Customer class and put it in the ExtraCharge class and the GroupExtra class?

Yes, it makes sense to keep it around in the GroupExtra class (you could simply calculate the corresponding charge in the constructor, but storing it is equally valid). However it certainly shouldn't go in the ExtraCharge class, which itself has no notion of the number of guests: remember you are using polymorphism here, so the details of the GroupExtra class (the fact that it gives extra charges based on the number of guests) shouldn't leak into the parent class ExtraCharge (which itself only provides a method to get the charge, which is implemented by the derived classes).

As you can see this is lots of material to digest to put into your design report: you should reflect on and discuss why separating and abstracting the different parts of the bill make it easier to maintain and extend the behaviour of the program, and so on.

About the specification, it's slightly vague but looking at it again, I don't think it matters about the room booking. If that is the case, is the Room class redundant?

I've attached the relevant part of the spec just in case you wanted a quick look.

Okay, looks like you're just meant to take an array of customers and calculate the bill for each. I assume the array of customers could come from anywhere, probably just a test dataset created manually with different types of customers. That makes sense.

Well, really, you would take an array of bookings in your design, which each contain a customer and the room the customer booked; you should probably check with your assignment if it's not actually the opposite that's expected, i.e. an array of customers that hold one or more active bookings, I don't know - both designs seem acceptable and use the object-oriented concepts you are meant to be using, so I don't see a big problem here.

In any case I would recommend wrapping up the design quickly and then proceeding to the implementation, because nothing clears up concepts better than actually implementing them ;) (you can still amend/improve the design as you go, of course, I imagine, unless you are meant to hand in each component separately)

 

[JamesBwoii]

Perfect! Is there any issues with the latest class diagram before I implement it?

View attachment 3964Thanks! :D

 

[Nono713]

I think you might need to add the different constructors to each (non-interface) class diagram, at least when not obvious, but I'm not 100% sure on that. Also check your arrows, Customer doesn't really inherit from Discount, I think you meant composition (aggregation) between Discount and Customer (and also ExtraCharge and Customer), right? Other than that, looks good!

 

[JamesBwoii]

Would you be able to explain something for me? Basically, I've got the ExtraCharge class with this code:

public class ExtraCharge {

	double charge;
	
	public double getCharge(){
		return charge;
	}
	
	public void calculateCharge(){
		
	}
	
}

and the class GroupExtra which inherits ExtraCharge:

public class GroupExtra extends ExtraCharge {

	Integer numberOfGuests;
	double extraCharge;
	
	public GroupExtra(Integer numberOfGuests){
		this.numberOfGuests = numberOfGuests;
	}
	
	public void calculateCharge(){
		extraCharge = 1+((numberOfGuests - 1) * 0.1);
	}
	
	public double getCharge(){
		return extraCharge;
	}
		
}

Is what I've done there the correct way to use super and subclasses?

 

[Nono713]

Would you be able to explain something for me? Basically, I've got the ExtraCharge class with this code:

public class ExtraCharge {

	double charge;
	
	public double getCharge(){
		return charge;
	}
	
	public void calculateCharge(){
		
	}
	
}

and the class GroupExtra which inherits ExtraCharge:

public class GroupExtra extends ExtraCharge {

	Integer numberOfGuests;
	double extraCharge;
	
	public GroupExtra(Integer numberOfGuests){
		this.numberOfGuests = numberOfGuests;
	}
	
	public void calculateCharge(){
		extraCharge = 1+((numberOfGuests - 1) * 0.1);
	}
	
	public double getCharge(){
		return extraCharge;
	}
		
}

Is what I've done there the correct way to use super and subclasses?

Well, at this rate ExtraCharge should really be an interface, since it doesn't implement anything by itself. Do you see why? Other than that, then the subclassing looks correct if this is Java, though usually you add an `@Override` tag just above the inherited functions to signal that they've been inherited from a parent class or interface. In this case that would be above the GroupExtra's calculateCharge() and getCharge() methods.

Here if you create a GroupExtra instance and assign it to a variable of type ExtraCharge, then calling its getCharge() method for instance will call the GroupExtra's one, and not the ExtraCharge's one, which is exactly what you want (by default, in Java all methods are virtual, so just implementing the same one in a child class automatically overrides the parent one). So it's correct.

I am still not quite sure why you want calculateCharge() and getCharge() to be two separate methods - wouldn't it be simpler to simply calculate the charge inside the getCharge() method directly? You could even in principle store it there on the first call so you don't have to calculate it again, but for such a simple calculation it doesn't seem worthwhile. What advantages do you perceive in a separate calculateCharge() method? More importantly, what happens if you call the getCharge() method before calling the calculateCharge() method?

 

[JamesBwoii]

I've made the changes you've suggested and think I've finished the actual logic for most of it. My Java is incredibly rusty and I've been brushing up but there's something I'm struggling a bit with. Basically how I tie for example, the CorporateCustomer, HotelBooking and CorporateDiscount classes together.

This is what they look like.

public class CorporateCustomer implements Customer {

	String name;
	Integer phone;
	String address;
	CorporateDiscount discount;
	
	public CorporateCustomer(String name, int phone, String address){
		this.name = name;
		this.phone = phone;
		this.address = address;
		this.discount = new CorporateDiscount(null);
		
	}
	
	@Override
	public void setName(String newName) {
		name = newName;
	}

	@Override
	public String getName() {
		return name;
	}

	@Override
	public void setPhone(Integer newPhone) {
		phone = newPhone;
	}

	@Override
	public Integer getPhone() {
		return phone;
	}

	@Override
	public void setAddress(String newAddress) {
		address = newAddress;
	}

	@Override
	public String getAddress() {
		return address;
	}

}

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.concurrent.TimeUnit;public class HotelBooking {
	
	Date dateFrom;
	Date dateTo;
	long stayLength;

	public HotelBooking(String dateFrom, String dateTo) throws ParseException{
		this.dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(dateFrom);
		this.dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(dateTo);
	}
	
	
	public void setDateFrom(String newDate) throws ParseException{
		dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(newDate);
	}
	
	public Date getDateFrom(){
		return dateFrom;
	}
	
	public void setDateTo(String newDate) throws ParseException{
		dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(newDate);
	}
	
	public Date getDateTo(){
		return dateTo;
	}
	
	public void setStayLength(){
		long diff = dateTo.getTime() - dateFrom.getTime();
	    stayLength = TimeUnit.DAYS.convert(diff, TimeUnit.MILLISECONDS);
	}
	
	public long getStayLength(){
		return stayLength;
	}

	
}

Sorry for the wall of code! Basically what I can't seem to do is create a new Corporate customer along with the dates of his stay, and pass that information to the CorporateDiscount class which uses the dates to calculate how many weekends that they stay on.

Thanks! :D :D

 

[Nono713]

Aha! That's because the Customer doesn't know the length of his stay: that information is in the HotelBooking, which is why the CorporateDiscount's getDiscount() method actually takes a date as parameter (instead of the date(s) being stored inside it). :)

 

[JamesBwoii]

Aha! That's because the Customer doesn't know the length of his stay: that information is in the HotelBooking, which is why the CorporateDiscount's getDiscount() method actually takes a date as parameter (instead of the date(s) being stored inside it). :)

Ah I thought I had posted the code to the CorporateDiscount class but it looks like I didn't.

import java.util.Calendar;public class CorporateDiscount implements Discount {

	Double discount;
	HotelBooking hotelBooking;
	long weekends;
	
	
	public CorporateDiscount(HotelBooking hotelBooking){
		this.hotelBooking = hotelBooking;
	}
	
	@Override
	public Double getDiscount() {
		Calendar cal1 = Calendar.getInstance();
	    Calendar cal2 = Calendar.getInstance();
	    cal1.setTime(hotelBooking.getDateFrom());
	    cal2.setTime(hotelBooking.getDateTo());
	    
		int numberOfDays = 0;
		while (cal1.before(cal2)) {
			if ((Calendar.SATURDAY != cal1.get(Calendar.DAY_OF_WEEK))
			&&(Calendar.SUNDAY != cal1.get(Calendar.DAY_OF_WEEK))) {
				numberOfDays++;
				cal1.add(Calendar.DATE,1);
		        }else {
		        	cal1.add(Calendar.DATE,1);
		        }
		    }
		    weekends = hotelBooking.calculateStayLength() - numberOfDays;		
		return discount;
	}

}

Is it correct to pass the HotelBooking class as a parameter to the CorporateDiscount class?

 

[Nono713]

By doing this you've coupled the CorporateDiscount class to the HotelBooking class, it can no longer function without it (and any breaking changes you make to the HotelBooking class will need to be mirrored in the CorporateDiscount class, and so on). Think a little bit more abstractly; look at the code inside your getDiscount() method: the HotelBookingSystem already has the HotelBooking, so it can already do much of the logic inside that method. In fact, the only logic inside this method which really depends on what kind of discount the customer has (here the CorporateDiscount) is those following lines:

if ((Calendar.SATURDAY != cal1.get(Calendar.DAY_OF_WEEK))
&&(Calendar.SUNDAY != cal1.get(Calendar.DAY_OF_WEEK))) {
    numberOfDays++;
    cal1.add(Calendar.DATE,1);
} else {
    cal1.add(Calendar.DATE,1);
}

Well, actually the code seems to be incomplete since you never assign the "discount" field to anything, but the idea is there: that code block above is pretty much the only thing the CorporateDiscount class stands for: to give a special discount on weekends, and a regular discount on weekdays.

Do you see how it would work if the HotelBookingSystem had the basic logic you currently have in the getDiscount() instead? It would instead ask the CorporateDiscount, for each day, "what discount should I use for this day" (of course, it wouldn't know that it's a CorporateDiscount, as far as it is concerned it's just a Discount). As opposed to the CorporateDiscount basically taking control of the whole thing, grabbing the HotelBooking and saying "here, I'll do your work for you" and forcing the CorporateDiscount to know about the internals of the HotelBooking, which increases coupling.

Hope that helps. The benefits are possibly not apparent to you right now, unfortunately, due to the fact that once the assignment is handed in you will never work on the hotel billing system again (whereas in real life that billing system would be maintained and upgraded for several years) so the difference between the two approaches is academic in that they both eventually work but one is "better design" (insert image of handwavy CS lecturer) but the main thing to keep in mind is that you want your abstractions to be as lightweight as possible to minimize complexity.

 

[JamesBwoii]

By doing this you've coupled the CorporateDiscount class to the HotelBooking class, it can no longer function without it (and any breaking changes you make to the HotelBooking class will need to be mirrored in the CorporateDiscount class, and so on). Think a little bit more abstractly; look at the code inside your getDiscount() method: the HotelBookingSystem already has the HotelBooking, so it can already do much of the logic inside that method. In fact, the only logic inside this method which really depends on what kind of discount the customer has (here the CorporateDiscount) is those following lines:

if ((Calendar.SATURDAY != cal1.get(Calendar.DAY_OF_WEEK))
&&(Calendar.SUNDAY != cal1.get(Calendar.DAY_OF_WEEK))) {
    numberOfDays++;
    cal1.add(Calendar.DATE,1);
} else {
    cal1.add(Calendar.DATE,1);
}

Well, actually the code seems to be incomplete since you never assign the "discount" field to anything, but the idea is there: that code block above is pretty much the only thing the CorporateDiscount class stands for: to give a special discount on weekends, and a regular discount on weekdays.

Do you see how it would work if the HotelBookingSystem had the basic logic you currently have in the getDiscount() instead? It would instead ask the CorporateDiscount, for each day, "what discount should I use for this day" (of course, it wouldn't know that it's a CorporateDiscount, as far as it is concerned it's just a Discount). As opposed to the CorporateDiscount basically taking control of the whole thing, grabbing the HotelBooking and saying "here, I'll do your work for you" and forcing the CorporateDiscount to know about the internals of the HotelBooking, which increases coupling.

Hope that helps. The benefits are possibly not apparent to you right now, unfortunately, due to the fact that once the assignment is handed in you will never work on the hotel billing system again (whereas in real life that billing system would be maintained and upgraded for several years) so the difference between the two approaches is academic in that they both eventually work but one is "better design" (insert image of handwavy CS lecturer) but the main thing to keep in mind is that you want your abstractions to be as lightweight as possible to minimize complexity.

I think I understand what you're saying just having trouble implementing it. Are you saying that I should move this code:

		Calendar cal1 = Calendar.getInstance();
	    Calendar cal2 = Calendar.getInstance();
	    cal1.setTime(hotelBooking.getDateFrom());
	    cal2.setTime(hotelBooking.getDateTo());
	    
		int numberOfDays = 0;
		while (cal1.before(cal2)) {
			if ((Calendar.SATURDAY != cal1.get(Calendar.DAY_OF_WEEK))
			&&(Calendar.SUNDAY != cal1.get(Calendar.DAY_OF_WEEK))) {
				numberOfDays++;
				cal1.add(Calendar.DATE,1);
		        }else {
		        	cal1.add(Calendar.DATE,1);
		        }
		    }
		    weekends = hotelBooking.calculateStayLength() - numberOfDays;

to the HotelBooking class and then just use a accessor so that CorporateDiscount class can access the weekends stayed in order to calculate the discount?

In order to do that wouldn't I still need to pass HotelBooking to the discount class so that it could access the number of weekends?

 

[Nono713]

I think I understand what you're saying just having trouble implementing it. Are you saying that I should move this code:

		Calendar cal1 = Calendar.getInstance();
	    Calendar cal2 = Calendar.getInstance();
	    cal1.setTime(hotelBooking.getDateFrom());
	    cal2.setTime(hotelBooking.getDateTo());
	    
		int numberOfDays = 0;
		while (cal1.before(cal2)) {
			if ((Calendar.SATURDAY != cal1.get(Calendar.DAY_OF_WEEK))
			&&(Calendar.SUNDAY != cal1.get(Calendar.DAY_OF_WEEK))) {
				numberOfDays++;
				cal1.add(Calendar.DATE,1);
		        }else {
		        	cal1.add(Calendar.DATE,1);
		        }
		    }
		    weekends = hotelBooking.calculateStayLength() - numberOfDays;

to the HotelBooking class and then just use a accessor so that CorporateDiscount class can access the weekends stayed in order to calculate the discount?

In order to do that wouldn't I still need to pass HotelBooking to the discount class so that it could access the number of weekends?

The CorporateDiscount's getDiscount() method takes a single date as a parameter and returns the appropriate discount for that date (depending on if it's a weekday or weekend). The HotelBooking class passes the date of each day of the stay to it (just the dates, nothing more) to calculate the discount applicable for that day.

 

[JamesBwoii]

The CorporateDiscount's getDiscount() method takes a single date as a parameter and returns the appropriate discount for that date (depending on if it's a weekday or weekend). The HotelBooking class passes the date of each day of the stay to it (just the dates, nothing more) to calculate the discount applicable for that day.

I tried doing what you suggested but not sure how passing only 1 dates works? I understand in practice how it can pass 1 date at a time and from that work out the days but I don't know how to do that. Instead I passed both beginning and end dates but did it slightly differently than I have done before. I've also used them to actually calculate a discount this time.

Is this right?

import java.util.Calendar;
import java.util.Date;public class CorporateDiscount implements Discount {

	Double discount;
	Integer weekends;
	Integer weekDays;
	
	
	@Override
	public Double getDiscount(Date dateFrom, Date dateTo) {
	    Calendar cal1 = Calendar.getInstance();
	    Calendar cal2 = Calendar.getInstance();
	    cal1.setTime(dateFrom);
	    cal2.setTime(dateTo);
	    
	    int weekendDays = 0;
	    int weekDays = 0;
	    
		while (cal1.before(cal2)) {
			if ((Calendar.SATURDAY == cal1.get(Calendar.DAY_OF_WEEK))
			||(Calendar.SUNDAY == cal1.get(Calendar.DAY_OF_WEEK))) {
				weekendDays++;
				cal1.add(Calendar.DATE,1);
		        }else {
		        	weekDays++;
		        	cal1.add(Calendar.DATE,1);
		        }
		    }
		    weekends = weekendDays;
		    weekDays = weekDays;
		    
		    discount = (weekends * 0.5) + (weekDays * 0.8);		
		    return discount;
	}
}

Thanks! :D

 

[Nono713]

That could work but the problem is I don't think you can add discounts like this. For instance in your implementation above, staying two weekends and three week days corresponds to a discount of 3.4? You'll need to separate the discounts, that is, in general you cannot derive a single discount value for all days. You might in this assignment since the flat rate is fixed, but it doesn't really work I think.

For the single date, what I was suggesting was as follows:

public class CorporateDiscount implements Discount {
    // ...

    @Override
    public Double getDiscount(Date date) {
        if ((date.get(Calendar.DAY_OF_WEEK) == Calendar.SATURDAY)) || (date.get(Calendar.DAY_OF_WEEK) == Calendar.SUNDAY))) {
            return 0.5; // weekend
        } else {
            return 0.8; // weekdays
        }
    }
}

And then inside the HotelBookingSystem you would loop over all days, and for each day ask for the discount for that specific day:

public Currency calculateBill(HotelBooking booking) {
    Calendar cal1 = Calendar.getInstance();
    Calendar cal2 = Calendar.getInstance();
    cal1.setTime(booking.getDateFrom());
    cal2.setTime(booking.getDateTo());

    Currency totalBill = 0.0; // I don't know what the Java Currency type is, so just assume its a double here

    while (cal1.before(cal2)) {
        // for each day of the stay for this booking... (here day = cal1)

        Currency billForThisDay = flatRate;

        // work out the discount...

        Discount discount = booking.getCustomer().getDiscount();

        if (discount != null) { // has discount?
            // what is the discount for this day?
            billForThisDay = billForThisDay * discount.getDiscount(cal1);
        }

        // same thing for extra charge and anything else needed to bill...

        totalBill = totalBIll + billForThisDay;

        cal1.add(Calendar.DATE,1); // go to the next day
}

    return totalBill;
}

As you can see, moving the "loop over each day in the booking" construct off to the HotelBookingSystem, and having it call into the other classes to get the information it needs. Is that clearer?

 

[JamesBwoii]

Ah yeah that's clearer. Just one thing, wouldn't the discount way I was using it work? If you stay 2 weekend days and 3 weekdays that equals 3.4. 3.4 multiplied by 100 = 340. If you calculate it manually you get 50 + 50 + 80 + 80 + 80 = 340.

Anyway, suppose it doesn't really matter anyway!

 

[JamesBwoii]

That could work but the problem is I don't think you can add discounts like this. For instance in your implementation above, staying two weekends and three week days corresponds to a discount of 3.4? You'll need to separate the discounts, that is, in general you cannot derive a single discount value for all days. You might in this assignment since the flat rate is fixed, but it doesn't really work I think.

For the single date, what I was suggesting was as follows:

public class CorporateDiscount implements Discount {
    // ...

    @Override
    public Double getDiscount(Date date) {
        if ((date.get(Calendar.DAY_OF_WEEK) == Calendar.SATURDAY)) || (date.get(Calendar.DAY_OF_WEEK) == Calendar.SUNDAY))) {
            return 0.5; // weekend
        } else {
            return 0.8; // weekdays
        }
    }
}

And then inside the HotelBookingSystem you would loop over all days, and for each day ask for the discount for that specific day:

public Currency calculateBill(HotelBooking booking) {
    Calendar cal1 = Calendar.getInstance();
    Calendar cal2 = Calendar.getInstance();
    cal1.setTime(booking.getDateFrom());
    cal2.setTime(booking.getDateTo());

    Currency totalBill = 0.0; // I don't know what the Java Currency type is, so just assume its a double here

    while (cal1.before(cal2)) {
        // for each day of the stay for this booking... (here day = cal1)

        Currency billForThisDay = flatRate;

        // work out the discount...

        Discount discount = booking.getCustomer().getDiscount();

        if (discount != null) { // has discount?
            // what is the discount for this day?
            billForThisDay = billForThisDay * discount.getDiscount(cal1);
        }

        // same thing for extra charge and anything else needed to bill...

        totalBill = totalBIll + billForThisDay;

        cal1.add(Calendar.DATE,1); // go to the next day
}

    return totalBill;
}

As you can see, moving the "loop over each day in the booking" construct off to the HotelBookingSystem, and having it call into the other classes to get the information it needs. Is that clearer?

Made a few changes but not sure what you've done in a few places. General consensus on the internet seems to be to use BigDecimal when dealing with money in Java so I've decided to use that. Made a few changes to the CorporateDiscount class you posted as I don't think it's possible to get the day from the Date data type but you have to create a new Calendar using that date so I've done that.

import java.math.BigDecimal;
import java.util.Calendar;
import java.util.Date;

public class CorporateDiscount implements Discount {

    @Override
    public BigDecimal getDiscount(Date date) {
    	Calendar cal = Calendar.getInstance();
    	cal.setTime(date);
    	if ((Calendar.SATURDAY == cal.get(Calendar.DAY_OF_WEEK)) || (Calendar.SUNDAY == cal.get(Calendar.DAY_OF_WEEK))) {
    		BigDecimal weekend = new BigDecimal(0.5);
            return weekend;
        } else {
        	BigDecimal weekday = new BigDecimal(0.8);
            return weekday; // weekdays
        }
    }

}

And in the HotelBooking class

import java.math.BigDecimal;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Currency;
import java.util.Date;
import java.util.concurrent.TimeUnit;public class HotelBooking {
	
	Date dateFrom;
	Date dateTo;
	long stayLength;
	long weekends;
	private Integer flatRate = 100;
	private BigDecimal totalBill;

	public HotelBooking(String dateFrom, String dateTo) throws ParseException{
		this.dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(dateFrom);
		this.dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(dateTo);
		calculateStayLength();
	}
	
	
	public void setDateFrom(String newDate) throws ParseException{
		dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(newDate);
	}
	
	public Date getDateFrom(){
		return dateFrom;
	}
	
	public void setDateTo(String newDate) throws ParseException{
		dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(newDate);
	}
	
	public Date getDateTo(){
		return dateTo;
	}
	
	public long calculateStayLength(){
		long diff = dateTo.getTime() - dateFrom.getTime();
	    stayLength = TimeUnit.DAYS.convert(diff, TimeUnit.MILLISECONDS);
	    return stayLength;
	}
	
	public BigDecimal calculateBill(HotelBooking booking) {
	    Calendar cal1 = Calendar.getInstance();
	    Calendar cal2 = Calendar.getInstance();
	    cal1.setTime(booking.getDateFrom());
	    cal2.setTime(booking.getDateTo());

	    BigDecimal totalBill =  new BigDecimal(0.0);
	    while (cal1.before(cal2)) {
	        // for each day of the stay for this booking... (here day = cal1)

	        BigDecimal billForThisDay = new BigDecimal(flatRate);

	        // work out the discount...

	        Discount discount = booking.getCustomer().getDiscount();

	        if (discount != null) { // has discount?
	            // what is the discount for this day?
	            billForThisDay = billForThisDay.multiply(discount.getDiscount(cal1));
	        }

	        // same thing for extra charge and anything else needed to bill...

	        totalBill = totalBill.add(billForThisDay);

	        cal1.add(Calendar.DATE,1); // go to the next day
	}

	    return totalBill;
	}
	
}

I don't get this line:

	        Discount discount = booking.getCustomer().getDiscount();

I don't have a getCustomer method in the Discount class? How exactly would that method work?

Thanks as always :D :D :D

 

[Nono713]

 Discount discount = booking.getCustomer().getDiscount();

Here we are just grabbing the Customer from the booking and getting to his discount via his getDiscount() method, i.e. equivalent to:

Customer customer = booking.getCustomer();
Discount discount = customer.getDiscount();

 

[JamesBwoii]

 Discount discount = booking.getCustomer().getDiscount();

Here we are just grabbing the Customer from the booking and getting to his discount via his getDiscount() method, i.e. equivalent to:

Customer customer = booking.getCustomer();
Discount discount = customer.getDiscount();

Sorry, I might be being stupid but I'm lost. There's no getCustomer() method in customer. In fact there is no getCustomer() method in any class?

 

[Nono713]

Sorry, I might be being stupid but I'm lost. There's no getCustomer() method in customer. In fact there is no getCustomer() method in any class?

Hum. The HotelBooking class in your diagram has a line connecting it to the customer, so I was under the impression every hotel booking was associated with a customer (like it's associated with a room). If that was not the case and I misread, then yes you might have to invert the logic a little bit (for instance if instead of bookings holding customers you have customers holding bookings, then instead of booking.getCustomer() you'd want customer.getBooking() and so forth). Either way the diagram is indeed missing something since you have a relationship between Customer and HotelBooking yet neither have methods that interact with the other (i.e. in the calculateBill() method, how is the billing system to know which customer is associated with a given hotel booking? that is the problem you are having right now)

Sorry for the confusion, does that clear things up a bit?

 

[JamesBwoii]

Hum. The HotelBooking class in your diagram has a line connecting it to the customer, so I was under the impression every hotel booking was associated with a customer (like it's associated with a room). If that was not the case and I misread, then yes you might have to invert the logic a little bit (for instance if instead of bookings holding customers you have customers holding bookings, then instead of booking.getCustomer() you'd want customer.getBooking() and so forth). Either way the diagram is indeed missing something since you have a relationship between Customer and HotelBooking yet neither have methods that interact with the other (i.e. in the calculateBill() method, how is the billing system to know which customer is associated with a given hotel booking? that is the problem you are having right now)

Sorry for the confusion, does that clear things up a bit?

Ah, I see. Ideally I'd like to follow my UML and connect it so that every hotel booking is connected with a customer.

I've looked back at my UML, which is slightly different from the last time I uploaded it, and can see that a the Customer class is associated to HotelBooking class with aggregation.

I've done some reading to find out how aggregation is implemented in Java and found this.

final class Car {

  private Engine engine;

  void setEngine(Engine engine) {
    this.engine = engine;
  }

  void move() {
    if (engine != null)
      engine.work();
  }
}

I've played around and can't seem to it so this line works though.

Discount discount = booking.getCustomer().getDiscount();

Any tips would be fantastic! :D

 

[Nono713]

Ah, I see. Ideally I'd like to follow my UML and connect it so that every hotel booking is connected with a customer.

I've looked back at my UML, which is slightly different from the last time I uploaded it, and can see that a the Customer class is associated to HotelBooking class with aggregation.

I've done some reading to find out how aggregation is implemented in Java and found this.

final class Car {

  private Engine engine;

  void setEngine(Engine engine) {
    this.engine = engine;
  }

  void move() {
    if (engine != null)
      engine.work();
  }
}

I've played around and can't seem to it so this line works though.

Discount discount = booking.getCustomer().getDiscount();

Any tips would be fantastic! :D

Can you show your latest UML diagram?

If you are going with "booking has a customer" then I'd simply pass the Customer to the booking's constructor (which connects the customer to the booking) and then add a getCustomer() method to the booking to retrieve it. Because here the HotelBooking doesn't have any functionality by itself (which is all in the HotelBookingSystem), it merely stores the customer that should be billed for the booking.

 

[JamesBwoii]

Can you show your latest UML diagram?

If you are going with "booking has a customer" then I'd simply pass the Customer to the booking's constructor (which connects the customer to the booking) and then add a getCustomer() method to the booking to retrieve it. Because here the HotelBooking doesn't have any functionality by itself (which is all in the HotelBookingSystem), it merely stores the customer that should be billed for the booking.

View attachment 4039

So then the HotelBooking constructor should look this this?

	public HotelBooking(String dateFrom, String dateTo, Customer customer) throws ParseException{
		this.dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(dateFrom);
		this.dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(dateTo);
		this.customer = customer;
	}

And then the HotelBooking also has a method called getCustomer(), but what is that returning? I assume it should essentially return what type of customer it is, so either individual, group or corporate but how do I go about doing that?

 

[Nono713]

So then the HotelBooking constructor should look this this?

	public HotelBooking(String dateFrom, String dateTo, Customer customer) throws ParseException{
		this.dateFrom = new SimpleDateFormat( "yyyy MM dd" ).parse(dateFrom);
		this.dateTo = new SimpleDateFormat( "yyyy MM dd" ).parse(dateTo);
		this.customer = customer;
	}

And then the HotelBooking also has a method called getCustomer(), but what is that returning? I assume it should essentially return what type of customer it is, so either individual, group or corporate but how do I go about doing that?

The constructor is correct. The getCustomer() method just returns this.customer as a Customer object, without any further work necessary: you don't need to differentiate between individual/group/corporate customers because all the differentiation is already done in the discount/charge objects you implemented.

 

[JamesBwoii]

The constructor is correct. The getCustomer() method just returns this.customer as a Customer object, without any further work necessary: you don't need to differentiate between individual/group/corporate customers because all the differentiation is already done in the discount/charge objects you implemented.

Ok, I've got the first line giving me no errors but the second one still is:

	        Customer customer = booking.getCustomer();
	        Discount discount = customer.getDiscount();

I can see on my UML diagram that the Discount class is connected to the Customer class by aggregation. So does this mean that the Discount class needs a Customer passed to it in it's constructor and then a getDiscount method or is it the other way around? Because the Discount class is just an interface that looks like this and I know interfaces can't have constructors so how do I connect it to the customer?

import java.math.BigDecimal;
import java.util.Calendar;
import java.util.Date;public interface Discount {

	public BigDecimal getDiscount(Date date);
	
}