Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CS2113-T11-1] HotelLite Manager #32

Open
wants to merge 824 commits into
base: master
Choose a base branch
from

Conversation

siewyangzhi
Copy link

This HotelLite Manager ChatBot aids the Hotel Manager with day-to-day hotel operations such as inventory management and housekeeping matters. Room status checks and customer satisfaction tracking are also made easier with the HotelLite Manager. It's designed for CLI users, so users can type in commands to do things like check inventory or check room status quickly. The hotel manager's workflow and the hotel's overall performance will be boosted by using this ChatBot

Copy link

@danbaterisna danbaterisna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good explanation of the design, but can be made clearer in some parts.


The objective of the `AddHousekeeperCommand` is to take in user input and spilt it into two parts which is the name and age of a Housekeeper. These details make up parts of the Housekeeper profile. With the name and age derived, this information will be added into a new Housekeeper object, which will be recorded into the list of housekeeper. The class diagram below depicts how the `AddHousekeeperCommand` interacts with other classes.

![class](team/falicia_addHousekeeperCommand/classAddHousekeeper.jpg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this diagram is too complicated for what it claims to do, since it shows lots of internal details when its goal is to show how AddHousekeeperCommand interacts with other classes. Consider focusing on important associations and only leaving in important attributes? (The same can be said for the AddItemCommand's class diagram.)

- `AddItemCommand#getItem` - Extracts the item saved within the AddItemCommand object.
- `AddItemCommand#execute` - Adds the item into the list of items.

![alt text](team/SiewYangZhi_addItemCommand/AddItemCommandSequenceDiagram.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sequence diagram is quite large. To help with reading, consider breaking it up into parts for each individual step?


### check room information by level

The checking room information by level mechanism is facilitated `CheckRoomByLevelCommand`. It extends command. Additionally, it implements the following operations:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think command should be monospaced, as in Command?

Comment on lines 28 to 43
Step 1: When the user enters the command `Add Item Toilet Roll / 15`, the `Duke` class would pass the user input to the `Command Parser` Class.

Step 2: The `Command Parser` Class would run the `parse` method and create an `AddItemCommand` object containing an `Item` object which is made up of the name of the item to add as well as its pax which are found within the user input.

Step 3: The `AddItemCommand` class would be passed back to the `Duke` class.

Step 4: The `execute` method of the `AddItemCommand` class would be run and the `ItemList` Object would be extracted from the `List Container` object and the `AddItemCommand` object would pass its Item object to the `ItemList` Object's `addItemToList` method.

Step 5: The `addItemToList` method would then call the `checkForItemDuplicates` method to check if there are any items within the item list with the same name as the item we are about to add.

Step 6:I f there are is an item with a matching name found, we would then call the `UI` object and execute the `UI`'s `printItemAlreadyInTheListErrorMessage` method which would print a message informing the user that the item the user wants to add is already found within the item list and hence nothing would be added. Step 7 and 8 are skipped.

Step 7: If there are no items with a matching name found, we would add the `Item` object to its ArrayList of `Item` objects called listOfItems. The `AddItemCommand` then call the `UI` and execute the `UI`'s `printAddItemAcknowledgementMessage` method which would print an acknowledgement message to the user informing him that the item has been added into the item list.

Step 8: The `ItemListFileManager` object would be called and we would pass the item list to its `writeItemListToFile` method to write the updated item list into the file ListFolder/ItemList.txt.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand what this enumeration is listing out. If this is the execution of an AddItemCommand, consider adding a few sentences explaining so. Would it also be better to add a sequence diagram to accompany this explanation?

__Step 1.__ The user launches the application. In the `Duke` class, an empty instance of the `AssignmentMap` class,
called `assignmentMap`, is created.

![Step 1](team/aiman_assignment/step1.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to italicize the Duke label here?

Also, if you're not adding any attributes to the classes, would it be fine to remove the boxes that would contain them? (So, only a box containing class names would be left.)
image

Copy link

@warrencxw warrencxw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, the segment under Design & Implementation looks well established. However, formatting between all the diagrams is inconsistent (different diagrams seem to be made with different softwares) and as such may result in difficulty in reading for the reader.


![Step 5](team/aiman_assignment/step5.png)

__Interaction__ To understand the interaction between objects of these classes in order to actually add an entry into the hash map inside an `AssignmentMap`, refer to the sequence diagram below.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider leaving a newline after the header "Interaction" to allow this segment to help it be clearer. Otherwise it may sound as if "Interaction" belongs to the sentence which does not appear to be the case.


__Interaction__ To understand the interaction between objects of these classes in order to actually add an entry into the hash map inside an `AssignmentMap`, refer to the sequence diagram below.

![Step Sequence Diagram](team/aiman_assignment/sequence.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues with the sequence diagram:

  1. Return arrows should simply contain the object it returns, return can be omitted.
  2. If nothing is returned, it can simply be a blank dotted arrow pointing back to the caller.
  3. Syntax of content for return arrows should be as follows: variableName: ClassName

Consider making the following changes for better clarity:

  1. Calling arrow for AssignmentMap#addAssignment requires a closing bracket in the text
  2. Return arrow for RoomList#getRoomList can simply be labelled rooms: ArrayList<Room>
  3. Return arrow for HousekeeperList#getHousekeeperList can simply be labelled hs: ArrayList<Housekeeper>
  4. Return arrow for AssignmentMap#houseKeeperExists should be found: boolean for type boolean. Additionally, the set of arrows for AssignmentMap#houseKeeperExists should be enlarged as it is very hard to read at this size.
  5. Return arrows from :AssignmentMap to ;AssignHousekeeperCommand and onwards can simply be blank.

Comment on lines 105 to 111
Step 1. The user launches the application. In the `Duke` class, an empty instance of the `SatisfactionList` class,
called `satisfactionList`, is created.

![Step 1 Object Diagram](team/alinazheng_addsatisfactioncommand_uml/AlinaZheng_AddSatisfaction_Step1ObjectDiagram.png)


Step 2. The user types the command `add satisfaction Bob 5`. In the `Duke` class, a `Command` object

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Step# be bolded as per above for consistency?

entity that created those instances, but those are not indicated in the diagram. Furthermore, for neatness, the
`User` entity should be the left-most entity, but that could not be formatted properly in the diagram.

![Sequence Diagram](team/alinazheng_addsatisfactioncommand_uml/AlinaZheng_AddSatisfaction_SequenceDiagram.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Consider changing the arrow from Duke to CommandParser to be a solid arrow rather than a dotted arrow as you are making a method call rather than returning from a method call.
Additionally, consider adding a : to all class/object entities in the sequence diagram, e.g. :CommandParser to follow the correct formatting.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Additionally, consider removing the entities at the bottom as well as showing the ends of the activation bars.

to verify if name has been recorded. If name is not in records, `HousekeeperList#addHousekeeper()` would be called to
add the housekeeper, `susan` and `33` into the housekeeperList.

![Sequence](team/falicia_addHousekeeperCommand/sequenceAddHousekeeperCommandv2.jpg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Consider aligning all arrows (e.g. arrowhead for methods should point to the tip of the activation bar) and offsetting each separate arrow to indicate order of occurrence (e.g. parse() occurs before AddHousekeeperCommand() and thus the arrow for AddHousekeeperCommand() should be offset slightly below the arrow of parse().


Step 5: The `addItemToList` method would then call the `checkForItemDuplicates` method to check if there are any items within the item list with the same name as the item we are about to add.

Step 6:I f there are is an item with a matching name found, we would then call the `UI` object and execute the `UI`'s `printItemAlreadyInTheListErrorMessage` method which would print a message informing the user that the item the user wants to add is already found within the item list and hence nothing would be added. Step 7 and 8 are skipped.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be Step 6: If ...?
Consider fixing the formatting issue in this line to help with readability.

Copy link

@YaxinJoy YaxinJoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall Structure is clear! Just exists some little problems I think.

Comment on lines 18 to 21
1. Command given from the user: `add housekeeper susan / 33`. This command meant to add a new housekeeper called susan who is 33 years old.
2. The parse method from `CommandParser` will run parse to create `AddHousekeeperCommand` and would be return to Duke.
3. The execute method in `AddHousekeeperCommand` will be performed. It first checks with the housekeeperList if the name of the housekeeper to be added has been recorded into the list. If it has not been recorded, housekeeperList would then add this new user into the records. Ui will be called to print a confirmation to show that the housekeeper has been entered into the list.
5. If housekeeper exist, the housekeeper profile would not be added into the list.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the same format of showing steps of the usage of a command.

Comment on lines 65 to 68
![Step 2](team/aiman_assignment/step2.png)

__Step 3.__ The `CommandParser` class replaces the `Assign Susan ## 301` in the user input with an empty string,
leaving just `Susan ## 301`. Then, an instance of `AssignHousekeeperCommand` is created which extends the class `Command`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the diagram of step2, consider changing the relationship between Duke and CommandParser from dependence to the association. I think it should be association if you are using the object of another class?

The objective of the AddItemCommand is to allow the user to add a new item to the list of items found within the inventory. It takes in the user input and spilt it up into two parts which are the name of the item to be added and its pax. These two information would then be used to create an Item Object and the Item Object would be saved into a list of items.

Below is the partial class diagram detailing the design of the Add Item Command Class as well as its interactions with the various other classes required to execute the Add Item Command.
![alt text](team/SiewYangZhi_addItemCommand/AddItemCommandClassDiagram.png)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the diagram of Add Item Command, I think one method named public UI in UI might need to delete public as it's already shown as + in the class diagram.

Comment on lines 252 to 253
## Product scope
### Target user profile
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding more info under Product Scope


Step 5: The `excute` method will then call `printRoom(int level, RoomList roomList)` method to print room information at target level.

![sequence diagram](team/xunyi_checkroombylevelcommand_uml/checkRoomByLevel_Squence.png)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding the activation bar in each class. Besides, some variable does not show its usage after assigning value to them (e.g., isValidLevel). Consider adding opt block for this variable.

Copy link

@allyfern72 allyfern72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many details were offered in the DG with a somewhat clear overall format. There were only minor infringements that the team could consider addressing.


The objective of the `AddHousekeeperCommand` is to take in user input and spilt it into two parts which is the name and age of a Housekeeper. These details make up parts of the Housekeeper profile. With the name and age derived, this information will be added into a new Housekeeper object, which will be recorded into the list of housekeeper. The class diagram below depicts how the `AddHousekeeperCommand` interacts with other classes.

![class](team/falicia_addHousekeeperCommand/classAddHousekeeper.jpg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea on representing the AddHousekeeperCommand using UML. Would it be clearer to focus more on the AddHousekeeperCommand aspect in the class diagram by removing other details (e.g. methods like printHousekeeperNoted())?

Comment on lines 18 to 21
1. Command given from the user: `add housekeeper susan / 33`. This command meant to add a new housekeeper called susan who is 33 years old.
2. The parse method from `CommandParser` will run parse to create `AddHousekeeperCommand` and would be return to Duke.
3. The execute method in `AddHousekeeperCommand` will be performed. It first checks with the housekeeperList if the name of the housekeeper to be added has been recorded into the list. If it has not been recorded, housekeeperList would then add this new user into the records. Ui will be called to print a confirmation to show that the housekeeper has been entered into the list.
5. If housekeeper exist, the housekeeper profile would not be added into the list.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the font formatting correct? Would denoting all the classes that appeared in the class diagram as code be more effective in relating this example to the class diagram?

Comment on lines 28 to 42
Step 1: When the user enters the command `Add Item Toilet Roll / 15`, the `Duke` class would pass the user input to the `Command Parser` Class.

Step 2: The `Command Parser` Class would run the `parse` method and create an `AddItemCommand` object containing an `Item` object which is made up of the name of the item to add as well as its pax which are found within the user input.

Step 3: The `AddItemCommand` class would be passed back to the `Duke` class.

Step 4: The `execute` method of the `AddItemCommand` class would be run and the `ItemList` Object would be extracted from the `List Container` object and the `AddItemCommand` object would pass its Item object to the `ItemList` Object's `addItemToList` method.

Step 5: The `addItemToList` method would then call the `checkForItemDuplicates` method to check if there are any items within the item list with the same name as the item we are about to add.

Step 6:I f there are is an item with a matching name found, we would then call the `UI` object and execute the `UI`'s `printItemAlreadyInTheListErrorMessage` method which would print a message informing the user that the item the user wants to add is already found within the item list and hence nothing would be added. Step 7 and 8 are skipped.

Step 7: If there are no items with a matching name found, we would add the `Item` object to its ArrayList of `Item` objects called listOfItems. The `AddItemCommand` then call the `UI` and execute the `UI`'s `printAddItemAcknowledgementMessage` method which would print an acknowledgement message to the user informing him that the item has been added into the item list.

Step 8: The `ItemListFileManager` object would be called and we would pass the item list to its `writeItemListToFile` method to write the updated item list into the file ListFolder/ItemList.txt.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the font formatting highlights how the different class diagram elements are being used. Nice work!


Step 2: The `Command Parser` Class would run the `parse` method and create an `AddItemCommand` object containing an `Item` object which is made up of the name of the item to add as well as its pax which are found within the user input.

Step 3: The `AddItemCommand` class would be passed back to the `Duke` class.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Duke be UI or is it another class altogether? It does not appear on the class diagram and is slightly confusing.


__Step 5.__ The `assignmentMap` looks for the appropriate `Room` object for `301` in the `RoomList` and then looks for the appropriate `Housekeeper` object for `Susan`. It then adds both to the hashmap contained inside itself.

![Step 5](team/aiman_assignment/step5.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if each step requires an object diagram. An object diagram is more suited for static representation rather than a dynamic one.

I think having the class diagram in the "design" section accompanied with the sequence diagram below would suffice.


Step 7: Changes in the list will be updated to file by calling `HousekeeperFileManager#save()` method.

![Sequence](team/falicia_deleteHousekeeperCommand/sequenceDeleteHousekeeper.jpg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there supposed to be method calls happening within the loop and alt frames? They look out of place being empty.
image


__Interaction__ To understand the interaction between objects of these classes in order to actually add an entry into the hash map inside an `AssignmentMap`, refer to the sequence diagram below.

![Step Sequence Diagram](team/aiman_assignment/sequence.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider to put addAssignment() a bit lower than execute()
Screen Shot 2022-03-31 at 10 29 12 AM


The following sequence diagram shows what would happen if the user typed `add satisfaction Bob 5`.

PLEASE NOTE: Due to the limitation of PlantUML, the lengths of the activation bars may not be correct, and

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to add a activation bar for User

Also, you may consider to use the same tool to draw the diagram
Screen Shot 2022-03-31 at 10 29 01 AM


__Interaction__ To understand the interaction between objects of these classes in order to actually add an entry into the hash map inside an `AssignmentMap`, refer to the sequence diagram below.

![Step Sequence Diagram](team/aiman_assignment/sequence.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, consider to put return from AssignmentMap to AssignmentHousekeeperCommand at the end of the activate bar of AssignmentMa
Screen Shot 2022-03-31 at 10 29 32 AM

to verify if name has been recorded. If name is not in records, `HousekeeperList#addHousekeeper()` would be called to
add the housekeeper, `susan` and `33` into the housekeeperList.

![Sequence](team/falicia_addHousekeeperCommand/sequenceAddHousekeeperCommandv2.jpg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to check this again as the parse() should point to the top barrier of the activation bar.
Screen Shot 2022-03-31 at 10 36 12 AM


Step 5: The `excute` method will then call `printRoom(int level, RoomList roomList)` method to print room information at target level.

![sequence diagram](team/xunyi_checkroombylevelcommand_uml/checkRoomByLevel_Squence.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to use activate bar
Screen Shot 2022-03-31 at 10 36 58 AM

zhengster and others added 25 commits April 6, 2022 22:23
Small edit to alinazheng.md
Changed name of PPP to github username
…dParser

Alina zheng delete refactor command parser
zhengster and others added 30 commits April 11, 2022 22:34
Running PR for adding and/or fixing docs before submission
Small fix to empty housekeeper rating case
…into Faliciaong-FinalTouchUp

# Conflicts:
#	ListFolder/event_file.txt
…into resolve-conflicts

# Conflicts:
#	ListFolder/event_file.txt
Delete design and implementation for DG
…ions

Update Item Exceptions to be more detailed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants