[v1.1][F12-B1] NUS F&T #73
Conversation
Many loops are verbose. This leads to longer lines of code that are not concise nor clean. Let's replace these verbose loops with lambda expressions to improve code quality.
The for loop in UniquePersonList#setPersons(List<Person>) is verbose and it can be simplified. Since lambda expressions cannot handle exceptions, this loop cannot be replaced by a lambda expression and needs to be simplified instead. Let's simplify this for loop using CollectionUtil#elementsAreUnique(Collection<?>).
…#805) Many loops are verbose. This leads to longer lines of code that are not concise nor clean. Let's replace these verbose loops with lambda expressions to improve code quality.
AddressBook#removePerson(Person) returns true if the person passed into this method can be found in the internal list, and throws a PersonNotFoundException otherwise. Returning a boolean is not required as the thrown exception would indicate that the person cannot be found. Let's update the return type for AddressBook#removePerson(Person) to void.
UniquePersonList#remove(Person) returns true if the person passed into this method can be found in the internal list, and false otherwise. It also throws PersonNotFoundException if a person is not found. Returning a boolean is not required as the exception is thrown before the value is returned. Let's update the return type for UniquePersonList#remove(Person) to void.
The MainWindow does not have an event handler to handle external requests (such as the close button or through the taskbar) to close the MainWindow. This may cause the app to not close properly since MainApp#stop() is not called as the ExitAppRequestEvent is not raised. MainApp#stop() is the method used to call Platform#exit() and System#exit(int) which closes our app and all remaining open windows. Thus, if MainApp#stop() is not called, the app may continue to run even after the MainWindow is closed, causing existing HelpWindows to remain open even when the user expects the app to stop. According to the life-cycle section in the documentation on JavaFX Application[1], the app will only finish and call MainApp#stop() when the app either calls Platform#exit() or when the last window has been closed. Thus, should there be any HelpWindows still opened, the app will continue running, causing those HelpWindows to remain open. Let's add an event handler for the onCloseRequest event in the root of MainWindow.fxml. This event will call MainWindow#handleExit(), which raises an ExitAppRequestEvent, when there is an external request to close the app. EmptyMainWindowHandle does not contain a close button, thus to test if an ExitAppRequestEvent was raised, we fire a WindowEvent#WINDOW_CLOSE_REQUEST event which will trigger the onCloseRequest event. [1] JavaFx documentation for Application: https://docs.oracle.com/javafx/2/api/javafx/application/Application.html
…move and removePerson (se-edu#836) AddressBook#removePerson(Person) returns true if the person passed into this method can be found in the internal list. UniquePersonList#remove(Person) returns true if the person passed into this method can be found in the internal list, and false otherwise. They both also throw PersonNotFoundException if a person cannot be found. Returning a boolean is not required as the thrown exception would indicate that the person cannot be found. Let's update the return type for AddressBook#removePerson(Person) and UniquePersonList#remove(Person) to void.
updates DeveloperGuide and UserGuide repo URL close #2
corrects typo for UserGuide & DeveloperGuide repo URL #3
Update repo url
|
Hi @A0155428B, your pull request title is invalid. For phase A, it should be in the format of For phase B, it should be in the format of Please follow the instructions given strictly and edit your title for reprocessing. Submit only one learning outcome per pull request (unless otherwise stated in instructions) and do remember to create your branches from the commit where the Note: this comment is posted by a bot. If you believe this is done in error, please create an issue at cs2103-pr-bot and add a link to this PR. |
There was a problem hiding this comment.
This is not a clean PR as you can tell it contains commits from people outside of your team organisation.
Have you guys work on the target user profile? Update the dev guide if you have done so.
Important : Do remember to give credit to code sources if your code is inspired from elsewhere. Just changing the variable names will not help - we will still be able to tell that it has been adapted. Please follow the guidelines as given in the learning resources!
Updates BrowserPanel to implement google search
…do, and Redo commands
update aboutus page
update user stories and add remark command
UI updates
Adds alias for some commands
|
The project scope recorded in the DG should be more specific. Is the current AB4 suitable for NUS students? If your product is catering to the same targeted users, what more can it bring? You should think through the product's end behavior and write the entire UG, for all the commands which you would want to implement. Then mark the features as future (v2.0) which you do not intend to implement in the project duration. We should be able to understand what is your product aiming to do from reading the User guide, which as of now is not the case. You should also indicate which features can be expected from which milestones. One more thing, remember to enable auto-publishing features for documentations (ReadMe, AboutUs, UG, DG etc...) |
|
I would want you to mention the following in your Developer guide
|
* Create command classes for job management features * Update Model API to support addJob and closeJob * Add new prefixes in CliSyntax to support adding of job * Rename command for remark * Add closeJob and addJob methods in ModelManager and AddCommandTest class * Rename attribute in Job class * Update AddJobCommand class * Update message in AddJobCommand class * UserGuide.adoc: update job adding feature format * Add parse method in ParserUtil class for VehicleNumber * Add new class AddJobCommandParser (incomplete) * Update authorship to classes * Update Logic and Model component to support job management * Remove all address parameter for add employee * Fix checkstyle error Fix checkstyle error * Update ui classes to remove address field * Rename message constant * Add toSet method in RemarkList class * Update CarviciM to accept jobs (non-persistent) * Update toString method of Job class * Fix checkstyle error * Fix checkstyle error * Update job adding to show on UI * Add test for AddJobCommand * Update authorship * Fix checkstyle errors Fix checkstyle error Fix checkstyle error Remove trailing whitespace Fix error * Rename attributes in classes and add new constructors * Update toSet method in RemarkList * Add two new storage classes for Job * Fix checkstyle error * Add new attribute in XmlSerializableAddressBook class for persitent data for job * Add method in ModelManager to support initialization of running job number * Fix checkstyle error * Fix missing methods
Initial PR.