[CS2113-T11-3] CliRental #29
Conversation
|
|
||
| public class CustomerList { | ||
|
|
||
| public static ArrayList<Customer> customers = new ArrayList<>(); |
There was a problem hiding this comment.
Inconsistent naming through the codebase: customers v.s. carsList.
It would be good if this is consistent throughout the codebase.
| String username = parameterContents[0]; | ||
| int age = Integer.parseInt(parameterContents[1]); | ||
| int contactNumber = Integer.parseInt(parameterContents[2]); | ||
| return new Customer(username , age, contactNumber ); |
There was a problem hiding this comment.
Be mindful about the horizontal whitespace you use throughout the codebase.
| return new Customer(username , age, contactNumber ); | |
| return new Customer (username , age, contactNumber); |
| return userInput; | ||
| } | ||
|
|
||
| public static boolean parse(String userInput) throws CliRentalException { |
There was a problem hiding this comment.
Boolean methods should start with is/has/etc.
Consider using more exceptions and asserts to assist with making sure the method runs successfully.
| System.out.println("Goodbye!"); | ||
| } | ||
|
|
||
| public static void getName(){ |
There was a problem hiding this comment.
Readers would normally expect a Name return type for a method named getName.
Consider renaming this method.
This issue is apparent throughout the codebase.
|
|
||
| public class Transaction { | ||
| private static int transactionCounter = 1; | ||
| private final String transactionId; |
There was a problem hiding this comment.
Final attributes should be named with SCREAMING_SNAKE_CASE.
| return false; | ||
| } | ||
|
|
||
| public static void addTx(Transaction transaction) { |
There was a problem hiding this comment.
Be consistent with the naming "Tx" and "transactions". The latter would be more appropriate.
| public class CustomerListTest { | ||
|
|
||
| @Test | ||
| public void testAddCustomer() { |
There was a problem hiding this comment.
Please follow the naming convention for tests.
docs/DeveloperGuide.md
Outdated
| @@ -2,21 +2,43 @@ | |||
|
|
|||
| ## Acknowledgements | |||
|
|
|||
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} | |||
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links | |||
| to the original source as well} | |||
There was a problem hiding this comment.
Could add a contents page here for easy reference for the DG
docs/DeveloperGuide.md
Outdated
|
|
||
|  |
There was a problem hiding this comment.
Should you add all the other classes in your main/java/car folder?
docs/DeveloperGuide.md
Outdated
|
|
||
| The following sequence diagram will explain the sequence of events after the user inputs an add-user command. | ||
|
|
||
|  |
There was a problem hiding this comment.
Nice use of numbers to explain sequence of events for long and complicated diagrams
| {Describe the value proposition: what problem does it solve?} | ||
| Our product, CliRental aims to allow quick adding of transactional data when renting out a car in a car rental | ||
| company/store. It also allows for the staff of the rental company to filter through the massive amount of transactions, | ||
| finding the transaction they are looking for easily with multiple filters. | ||
|
|
||
| ## User Stories |
There was a problem hiding this comment.
Could update these sections for the final version as they are needed
| start of the program. The carData.txt will be created if it does not exist at the start of program and its data will be | ||
| loaded if the file exist. | ||
|
|
||
|  | ||
|
|
There was a problem hiding this comment.
I would consider adding a few more UML diagrams if possible.
follow the format of the AB3
https://se-education.org/addressbook-level3/UserGuide.html
There was a problem hiding this comment.
Good attempt on updating your DG.
There are small issues pertaining your diagrams, please make sure that the diagrams communicate how your software is implemented.
Some information are missing due to lack of architecture / class diagrams.
Do complete your DG - some sections are empty.
docs/DeveloperGuide.md
Outdated
|
|
||
| ## Design & implementation | ||
|
|
||
| {Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.} | ||
| The following is our class diagram for our whole project. |
There was a problem hiding this comment.
An architecture diagram would be more suitable to describe the whole project.
This class diagram can be used to discuss how Customer class is being used in the project.
CliRental class seems to be very out of place here.
Do take note of the usage "Void" v.s. "void", "String" v.s. "string".
Do include multiplicity as well.
The class diagram seems to suggest aggregation instead of composition.
docs/DeveloperGuide.md
Outdated
|
|
||
| The following sequence diagram will explain the sequence of events after the user inputs an add-user command. | ||
|
|
||
|  |
There was a problem hiding this comment.
Good attempt on the sequence diagrams.
However, there may be too many information within the diagram. Perhaps omit details that involve the Java API e.g. method calls to ArrayList.
|
|
||
| The following sequence diagram illustrates a **valid** `add-car` operation: | ||
|
|
||
|  |
There was a problem hiding this comment.
Is there a reason why there are different software for creating sequence diagrams?
|
|
||
| The following class diagram shows the interaction between the classes involved: | ||
|
|
||
|  |
There was a problem hiding this comment.
It would be better to explain the classes involved first before showing sequence diagrams.
Multiplicities can be improved to both sides of an association.
Double-check with the composition: If CarList is destroyed, why would instances of Cars be destroyed too?
Double-check with dependencies: Does CarParser use CarList?
|
|
||
|  | ||
|
|
||
| Step 5: **Optionally**, the user can also execute |
There was a problem hiding this comment.
I like the explanation of this process in steps, it helps to make it more readable.
|
|
||
|
|
||
| Step 3: A customer decides to rent a car from the company. The user then uses the application to track | ||
| and record the transaction details. The user executes the `add-tx ...` command to add |
There was a problem hiding this comment.
Perhaps it would be better to have an example for this command, as it is not mentioned before in this document.
|
|
||
| Example: | ||
|
|
||
|  |
There was a problem hiding this comment.
Perhaps it would be better to use the same software for creating sequence diagrams to ensure consistency.
docs/DeveloperGuide.md
Outdated
|
|
||
|  |
There was a problem hiding this comment.
Perhaps it would be better to leave out the minor utility methods such as the getters and setters as the class diagram does look quite cluttered.
docs/DeveloperGuide.md
Outdated
|
|
||
| The following sequence diagram will explain the sequence of events after the user inputs an add-user command. | ||
|
|
||
|  |
There was a problem hiding this comment.
Perhaps line 14 should be dotted instead? Given that it shows the function CustomerException returning to the activation bar of CustomerParser.
Seems like the arrow for line 21 is in the wrong direction.
docs/DeveloperGuide.md
Outdated
|
|
||
|  | ||
|
|
||
| 2. `Creating Car file and loading file` |
There was a problem hiding this comment.
Seems like there was an error when rendering the document, it should be 2. instead of 1.
…customer has a rental for a new add-tx rental
Update adding of transaction , car , customer without printing info
…arseIntoCar' to throw the appropriate exception
Improve A-AddRental and Transaction_ID generation
…5/tp into A-SaveFileDocumentation
Update formatting
images of ryan and aaron
Updated ug and ppp
Slight modification of UG, DG, and upload team members' photos
update ppp and dg
update ppp
update ppp
Update AboutUs.md - Final PR
Fix Kenneth's PPP link error
CliRental helps manage a car rental business that handles large volumes of data, making traditional pen-and-paper methods inefficient for managers. This app is designed specifically for car rental managers, offering a Command Line Interface (CLI) that enables quick data entry and efficient tracking of customers, cars, and transactions.