-
Notifications
You must be signed in to change notification settings - Fork 103
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-F11-1] Property Rental Manager #16
base: master
Are you sure you want to change the base?
[CS2113-F11-1] Property Rental Manager #16
Conversation
docs/DeveloperGuide.md
Outdated
### PairingList | ||
|
||
```PairingList``` facilities that pair and unpair commands by storing client-property pairs. | ||
|
||
When client rents a property, the client and property form a pair. | ||
|
||
* ```PairingList``` uses a hash map to represent these client-property pairs, where the key is a ```Client``` object | ||
and the value is a ```Property``` object. | ||
* A hash map is chosen due to its constant time lookup performance, making it efficient at querying the property that a | ||
client is renting. | ||
* Also, the Java HashMap prevents duplicate keys, which dovetails nicely with the fact that real-life tenants only have | ||
one place of residence at any time. | ||
|
||
#### Pair | ||
|
||
The sequence diagram for the pair command is called is shown below: | ||
|
||
![PairingList Add Pair Sequence Diagram](diagrams/PairingListAddPairSD.png) | ||
|
||
**NOTE**: Some self-invocated calls have been omitted because this diagram emphasises cross-class method calls. | ||
|
||
The pair command takes in user input of the format: | ||
``` | ||
pair ip/PROPERTY_INDEX ic/CLIENT_INDEX | ||
``` | ||
where ```PROERTY_INDEX``` and ```CLIENT_INDEX``` must be positive integers which are indexes present in ```ClientList``` | ||
and ```PropertyList``` if their private arrays were 1-indexed. | ||
|
||
How the pair command works: | ||
1. The user input for a pair command is first parsed by ```Parser``` (specifically, ```ParsePair```). | ||
2. ```ParsePair``` checks the user input for formatting mistakes such as missing flags and wrong flag orders. | ||
3. ```ParsePair``` also calls helper methods in ```PairingList``` to check that the pairing client and property indexes | ||
exists. Also, the client and property must not be already paired. The client must not be renting any property | ||
presently as well. | ||
4. After passing all these checks, the program fetches the desired```Property``` and ```Client``` from | ||
```PropertyList``` and ```ClientList```. | ||
5. The ```Property``` and ```Client``` objects are inserted as a pair into the hashmap of ```PairingList```. | ||
|
||
#### Unpair | ||
|
||
The unpair command takes in user input of the format: | ||
``` | ||
unpair ip/PROPERTY_INDEX ic/CLIENT_INDEX | ||
``` | ||
where ```PROERTY_INDEX``` and ```CLIENT_INDEX``` must be positive integers which are indexes present in ```ClientList``` | ||
and ```PropertyList``` if their private arrays were 1-indexed. | ||
|
||
|
||
How the unpair command works: | ||
1. The user input for a pair command is first parsed by ```Parser``` (specifically, ```ParseUnpair```). | ||
2. ```ParseUnpair``` checks the user input for formatting mistakes such as missing flags and wrong flag orders. | ||
3. ```ParseUnpair``` also calls helper methods in ```PairingList``` to check that the pairing client and property | ||
indexes exist, and that the client-property pair exist in ```PairingList```. | ||
4. After passing all these checks, the ```PairingList``` deletes the hashmap entry in ```clientPropertyPairings``` | ||
which contains the client-property pair. | ||
|
||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done diagram!
docs/DeveloperGuide.md
Outdated
There are 5 different classes, that each inherit from the abstract Command class. The commands read information from | ||
the PropertyList and ClientList classes respectively, and display using the Ui class, making use of the objects of | ||
these classes. The Commands which display all the information - i.e. CommandListClients, CommandListProperties, and | ||
CommandListEverything read and display using loops inside the overriden execute() method itself. The Commands which | ||
display selected information - i.e. CommandListClientsWithTags and CommandListPropertiesWithTags use their private | ||
methods to display their information. The class structure is as follows - | ||
![ListClassDiagram](diagrams/ListClassDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diagram is too small. Please enlarge it. 😃
Also, you can remove the C and A logos beside the classes and abstractions.
docs/DeveloperGuide.md
Outdated
![Add Client Sequence Diagram](diagrams/AddClientSequenceDiagram.JPG) | ||
<p align="center"> | ||
Add Client Sequence Diagram | ||
</p> | ||
|
||
![Add Property Sequence Diagram](diagrams/AddPropertySequenceDiagram.JPG) | ||
<p align="center"> | ||
Add Property Sequence Diagram | ||
</p> | ||
|
||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence diagram is well done! But please enlarge the font. 😆
docs/DeveloperGuide.md
Outdated
The above is an example for CommandListProperties. It reads from PropertyList. Then, it displays each line | ||
using the displayOneProperty function in Ui. | ||
|
||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your diagrams are well done and detailed, but please enlarge the fonts. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, well done and detailed developer guide. However, there seems to be a lack of explanation on design decisions. The DG explains how things are done and less so on why things are done that way.
docs/DeveloperGuide.md
Outdated
- Single owner unit (Shared ownership will be registered under one owner's name) | ||
- Unable to calculate tax payment | ||
___ | ||
## User Stories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting for table looks off on render
The following *sequence diagram* shows how the **delete client** operation works, showcasing the | ||
```ClientList#deleteClient()``` method. | ||
|
||
![Delete Client Sequence Diagram](diagrams/DeleteClientSD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lifeline for deletedProperty does not look correctly shown
docs/DeveloperGuide.md
Outdated
|
||
The sequence diagram for the pair command is called is shown below: | ||
|
||
![PairingList Add Pair Sequence Diagram](diagrams/PairingListAddPairSD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sequence diagram attempts to explain too much. In fact, sequence diagrams should only be used to aid the explanation of concepts difficult to grasp, and not depict the entire program function. Suggest using just a small portion where program sequence is not simply linear.
docs/DeveloperGuide.md
Outdated
are checkers to ensure that a valid command has been entered. ParseListClient and ParseListProperty also determine | ||
if tags have been entered, and if those tags are valid. | ||
``` | ||
case COMMAND_LIST: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk of code is confusing and does not seem to help with explanation. Furthermore, some aspects of the code is also not explained.
docs/DeveloperGuide.md
Outdated
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.} | ||
### Architecture | ||
![Software Architecture Diagram](diagrams/ArchitectureDiagram.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lacking explanation on design decision, especially when the components look as interconnected as it is (high coupling).
docs/DeveloperGuide.md
Outdated
Given below is an example scenario on how add client/property behaves at each step. | ||
|
||
|
||
- **Step 1**: The user executes ```add -client n/NAME c/CONTACT_NUMBER e/EMAIL b/BUDGET_MONTH``` or ```add -property n/NAME a/ADDRESS p/PRICE t/TYPE```. Depending on `add -client` or `add -property` specified, a `Parser` object of type `ParseAddClient` or `ParseAddProperty` is created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider giving high level and more easily readable steps. Since methods are not individually explained, perhaps there is no need to quote specific methods.
docs/DeveloperGuide.md
Outdated
- Single owner unit (Shared ownership will be registered under one owner's name) | ||
- Unable to calculate tax payment | ||
___ | ||
## User Stories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/DeveloperGuide.md
Outdated
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.} | ||
### Architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/DeveloperGuide.md
Outdated
The following is a simple class diagram of the three classes: | ||
<p align="center"> | ||
|
||
![](diagrams/ParseAddRelatedClassesDiagram.jpg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
Also, most of the sub-methods are used to perform validations on the extracted details. These methods are implemented via regex pattern checker. | ||
|
||
- Client: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following *class diagram* shows all the classes involved in the **delete client/property** operation | ||
and their relationships. | ||
|
||
![Delete Client/Property Class Diagram](diagrams/DeleteClientPropertyCD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following *sequence diagram* shows how the **delete property** operation works, showcasing the | ||
```PropertyList#deleteProperty()``` method. | ||
|
||
![Delete Property Sequence Diagram](diagrams/DeletePropertySD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following *sequence diagram* shows how the **delete client** operation works, showcasing the | ||
```ClientList#deleteClient()``` method. | ||
|
||
![Delete Client Sequence Diagram](diagrams/DeleteClientSD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following *sequence diagram* shows how the **delete client** operation works, showcasing the | ||
```ClientList#deleteClient()``` method. | ||
|
||
![Delete Client Sequence Diagram](diagrams/DeleteClientSD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LIfeline is not drawn fully, also not sure if currentListSize-- should be included...
The following *sequence diagram* shows how the **delete property** operation works, showcasing the | ||
```PropertyList#deleteProperty()``` method. | ||
|
||
![Delete Property Sequence Diagram](diagrams/DeletePropertySD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lifeline is not drawn fully, not sure if currentListSize-- should be included...
|
||
The text file of which Client, Property and Pairing is being stored is `client.txt`, `property.txt` and `pairing.txt` | ||
respectively. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all the method calls in the diagram can be more readable by dropping the "this."
docs/DeveloperGuide.md
Outdated
|
||
The sequence diagram for the pair command is called is shown below: | ||
|
||
![PairingList Add Pair Sequence Diagram](diagrams/PairingListAddPairSD.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence diagram is too complicated, makes it hard to track what is going on.
throw new UndefinedSubCommandTypeException(MESSAGE_INCORRECT_LIST_DETAILS); | ||
} | ||
``` | ||
This block of code is part of ParseManager. It determines the type of list operation(-client, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be better represented using a sequence diagram rather than showing the source code
Expand test coverage for Storage feature
Add test code to cover the branch for find feature
Improve coding standards for Storage and ParseManager
…ress-Duplication-Bug Fix case insensitive address duplication bug
Fix incorrectly rendered tables in UG and DG
…into developer-guide-update
Optimize add-related error messages
Fix code quality issues
Update PPP
Add page break for PPP (OVReader)
Update manual testing section in DG
Add test code for Delete and Check
Remove icon for Add Command related class diagram in DeveloperGuide.md
Fix minor UG and DG issues
Fix rendering issue where image does not appear on Github Pages
Add new line after AddClient and AddProperty image
Update Delete Class Diagram
Add Singapore context description to UG and README
…nput-Bug Fix valid input space bug along with some coding violations
Fix bug that cause program to crash when backslash is placed after command
Property Rental Manager (PRM) is a desktop application that helps property agents manage, filter and monitor single owner rental units for appropriate tenants. This application uses Command Line Interface (CLI) and is able to display information quickly with minimal latency.