Update UG/DG and undo for training #188

junhuplim · 2019-11-02T12:25:16Z

No description provided.

* 'master' of https://github.com/AY1920S1-CS2103T-T12-3/main: Update picture paths for UG Update UG styling Fix failing tests Abstract parseDate method in ParserUtil Fix trailing whitespace Add Ui.png Update UG and dot indicator colour to green Make event view background white Allow feature box to switch to view events when event is added Enable scrolling for viewing events Add sample Records Updated userguide Update sample persons to match athlete profiles Remove unused import Correct command prompts Update UI image with populated training and performance Update UI picture Edit UserGuide Update version

coveralls · 2019-11-02T12:28:02Z

Pull Request Test Coverage Report for Build 259

0 of 0 changed or added relevant lines in 0 files are covered.
56 unchanged lines in 22 files lost coverage.
Overall coverage decreased (-0.7%) to 55.194%

Files with Coverage Reduction	New Missed Lines	%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/AddCommand.java	1	85.71%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/AttendanceCommand.java	1	0.0%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/CalendarCommand.java	1	75.0%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/ClearCommand.java	1	75.0%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/DeleteCommand.java	1	50.0%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/DeletePersonCommand.java	1	92.86%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/EditCommand.java	1	97.5%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/EventCommand.java	1	84.62%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/ExitCommand.java	1	50.0%
file:/home/travis/build/AY1920S1-CS2103T-T12-3/main/src/main/java/seedu/address/logic/commands/FilterCommand.java	1	90.0%

Totals
Change from base Build 257:	-0.7%
Covered Lines:	1562
Relevant Lines:	2830

💛 - Coveralls

shawnlsj97 · 2019-11-05T13:50:03Z

src/main/java/seedu/address/logic/commands/RedoCommand.java

-        + "undone any commands. As such, you are unable to redo any commands.";
+    public static final String MESSAGE_FAILURE_EMPTY_STACK = "Redo Command Failure: No available "
+        + "commands to be redone.";
+    public static final String MESSAGE_FAILURE = "Redo Command Failure:"


good improvement in the error message. just a point regarding grammar: should be "an" instead of "a"

shawnlsj97 · 2019-11-05T13:51:10Z

src/main/java/seedu/address/logic/commands/RedoCommand.java

+        } else {
+            Command redoneCommand = model.redo();
+            if (redoneCommand instanceof TrainingCommand) {
+                return new CommandResult(MESSAGE_SUCCESS, ((TrainingCommand) redoneCommand).getDate(), model);


good alteration of the commandresult to refresh the calendar to include the latest changes.

shawnlsj97 · 2019-11-05T13:51:49Z

src/main/java/seedu/address/model/ModelManager.java

-            Training undoneTraining = this.attendance.getTrainings().remove(lastIndex);
+            AthletickDate dateOfTraining = ((TrainingCommand) undoneCommand).getDate();
+            Training undoneTraining = this.attendance.getTrainingOnDate(dateOfTraining);
+            this.attendance.getTrainings().remove(undoneTraining);


good update to account for the new implementation of attendance

* 'master' of https://github.com/AY1920S1-CS2103T-T12-3/main: Update for checkstyle Update calendar command error message to tell user range of month and year command Make entire command box clickable, resize command and result box Make long fields wrap Edit clear command to refresh calendar, update tests as well Wrap text to allow long event names to be seen fully Update Model stubs Enable clearing of performance data Adhere to CheckStyle Edit to pass test cases Correct error message Enable deleting athletes by -p flag (test cases not passed) Introduce flags

* 'master' of https://github.com/AY1920S1-CS2103T-T12-3/main: Add ability to replace existing trainings Update InformationDisplay to include gender Edit ClearCommand to delete Attendance data Add function to delete training Add template for test classes Add View Attendance command Add tests to Attendance and Training Edit calendar display to show training attendance in alphabetical order # Conflicts: # src/main/java/seedu/address/logic/commands/TrainingCommand.java

hellodommy

Good job on the improvements in Undo and Redo Junhup!

There are just a few grammatical errors and inconsistencies in the UG/DG.

Also, do consider making the UndoCommand and RedoCommand success more specific (eg. what in the command has been undone) for more clarity.

hellodommy · 2019-11-05T18:29:23Z

docs/DeveloperGuide.adoc


-Step 2. The user executes `delete 5` command to delete the 5th person in the address book. The `delete` command calls `Model#commitAddressBook()`, causing the modified state of the address book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted address book state.
+Step 2. The user executes the delete 3 command to delete the 3rd person in the Athletick list. The delete command will be pushed into the commands stack in and the new state of the address book (after executing the delete command) will be pushed to the addressBooks stack as well.


Is there a reason for addressBook and addressBooks being used? There is some inconsistency here.

hellodommy · 2019-11-05T18:31:20Z

docs/DeveloperGuide.adoc


-Step 6. The user executes `clear`, which calls `Model#commitAddressBook()`. Since the `currentStatePointer` is not pointing at the end of the `addressBookStateList`, all address book states after the `currentStatePointer` will be purged. We designed it this way because it no longer makes sense to redo the `add n/David ...` command. This is the behavior that most modern desktop applications follow.
+How the redo feature works is very similar to how to undo feature work. The difference is that redo utilises the undoneCommands stack and the undoneAddressBook stack. As such, you can also refer to the diagrams in the Undo Implementation.


Minor edit to

How the redo feature works is very similar to how the undo feature works.

hellodommy · 2019-11-05T18:31:54Z

docs/DeveloperGuide.adoc

 ** Cons: May have performance issues in terms of memory usage.
 * **Alternative 2:** Individual command knows how to undo/redo by itself.
 ** Pros: Will use less memory (e.g. for `delete`, just save the person being deleted).
 ** Cons: We must ensure that the implementation of each individual command are correct.

-===== Aspect: Data structure to support the undo/redo commands
+====== Reason why we chose alternative 1:
+Even though the memory usage of Alternative 2 is lesser, we do not feel that the benefit of the usage of lesser memory outweigh the cost of implementing the alternative.


Minor edit to

outweighs

okay thank you!

hellodommy · 2019-11-05T18:34:34Z

src/main/java/seedu/address/logic/commands/AddCommand.java

@@ -74,4 +74,8 @@ public boolean equals(Object other) {
                || (other instanceof AddCommand // instanceof handles nulls
                && toAdd.equals(((AddCommand) other).toAdd));
    }
+    @Override
+    public String toString() {
+        return "Add Person Command";


If you included the Person details here (eg. maybe even just the Name), could it be possible to include more details in the UndoCommand success other than just Add Person Command undone?

Same for the other commands that have specific details (eg. Delete and Edit), so the user is more clear on what in particular has been undone.

I'll give it a shot

hellodommy · 2019-11-05T18:35:34Z

src/main/java/seedu/address/logic/commands/RedoCommand.java

-        + "undone any commands. As such, you are unable to redo any commands.";
+    public static final String MESSAGE_SUCCESS = "Redo ";
+    public static final String MESSAGE_FAILURE_EMPTY_STACK = "Redo Command Failure: No available "
+        + "commands to be redone.";


Consider changing to No available commands to redo.

hellodommy · 2019-11-05T18:38:10Z

src/main/java/seedu/address/logic/commands/RedoCommand.java

+                return new CommandResult(MESSAGE_SUCCESS + redoneCommand
+                    + " Success!", ((TrainingCommand) redoneCommand).getDate(), model);
+            } else {
+                return new CommandResult(MESSAGE_SUCCESS + redoneCommand


Consider showing the user what command has been redone for more clarity.

hellodommy · 2019-11-05T18:39:17Z

src/main/java/seedu/address/logic/commands/UndoCommand.java

+            return new CommandResult(MESSAGE_SUCCESS + undoneCommand
+                + " Success!", ((TrainingCommand) undoneCommand).getDate(), model);
+        } else {
+            return new CommandResult(MESSAGE_SUCCESS + undoneCommand


By editing the toString of the commands, perhaps this MESSAGE_SUCCESS can be more personalised.

hellodommy · 2019-11-05T18:42:20Z

src/main/java/seedu/address/ui/InformationDisplay.java

@@ -59,7 +59,7 @@ public InformationDisplay(Person selectedPerson, String attendance) {
        address.setText(this.person.getAddress().value);
        address.setPrefWidth(150);
        address.setWrapText(true);
-        gender.setText(this.person.getGender().genderOfPerson);
+        //gender.setText(this.person.getGender().genderOfPerson);


@ChangUo79 remember to implement this in the FXML, otherwise this will be just dead code 😵.

…ds undo/redo implementation to similar to addressbook

* 'master' of https://github.com/AY1920S1-CS2103T-T12-3/main: (30 commits) Comply with checkstyle Updated for ModelStub Updated InformationDisplay Updated for performance display for selectCommand Test equality for Flag Adhere to CheckStyle Test DeleteEventCommand Test getFlag in DeleteCommandParser Make constructor public to allow for testing Test type parsers in DeleteCommandParser Fix EditCommand to change person details in training records Show generic message usage when no arguments are supplied Integrate DeleteTraining error message Recognise -t as a valid flag Add basic tests for Flag Updated to display performance Adhere to CheckStyle Modify to pass test cases Fix Flag toString error Give specific error messages for invalid inputs ... # Conflicts: # src/main/java/seedu/address/MainApp.java # src/main/java/seedu/address/model/training/Training.java # src/main/java/seedu/address/ui/InformationDisplay.java

* 'master' of https://github.com/AY1920S1-CS2103T-T12-3/main: Fix checkstyle errors Rename all "addressbook" to "athletick" Edit DeveloperGuide # Conflicts: # src/main/java/seedu/address/MainApp.java # src/main/java/seedu/address/logic/LogicManager.java # src/main/java/seedu/address/model/Model.java # src/main/java/seedu/address/model/ModelManager.java # src/main/java/seedu/address/model/history/HistoryManager.java

junhuplim added 4 commits November 1, 2019 13:34

update UG DG and Undo for Training Command

df4df79

update user guide on gender and undo/redo

7c85680

Update redo to only execute after undo;

236d17c

shawnlsj97 reviewed Nov 5, 2019

View reviewed changes

junhuplim added 6 commits November 5, 2019 23:17

comment out gender ui bug

1354973

Specify undo and redo commands

44c0300

checkstyles

fb59414

undo add training command

0c81407

hellodommy reviewed Nov 5, 2019

View reviewed changes

junhuplim added 6 commits November 6, 2019 16:35

update undo/redo command success message, and change trainings comman…

91383da

…ds undo/redo implementation to similar to addressbook

Undo Edit success

e50f306

checkstyles

8282db0

checkstyles

9246e94

junhuplim merged commit 2c19534 into AY1920S1-CS2103T-T12-3:master Nov 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update UG/DG and undo for training #188

Update UG/DG and undo for training #188

junhuplim commented Nov 2, 2019

coveralls commented Nov 2, 2019 •

edited

shawnlsj97 Nov 5, 2019

shawnlsj97 Nov 5, 2019

shawnlsj97 Nov 5, 2019

hellodommy left a comment

hellodommy Nov 5, 2019

hellodommy Nov 5, 2019

junhuplim Nov 6, 2019

hellodommy Nov 5, 2019

junhuplim Nov 6, 2019

hellodommy Nov 5, 2019

junhuplim Nov 6, 2019

hellodommy Nov 5, 2019

hellodommy Nov 5, 2019

hellodommy Nov 5, 2019

hellodommy Nov 5, 2019


		Step 2. The user executes `delete 5` command to delete the 5th person in the address book. The `delete` command calls `Model#commitAddressBook()`, causing the modified state of the address book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted address book state.
		Step 2. The user executes the delete 3 command to delete the 3rd person in the Athletick list. The delete command will be pushed into the commands stack in and the new state of the address book (after executing the delete command) will be pushed to the addressBooks stack as well.


		Step 6. The user executes `clear`, which calls `Model#commitAddressBook()`. Since the `currentStatePointer` is not pointing at the end of the `addressBookStateList`, all address book states after the `currentStatePointer` will be purged. We designed it this way because it no longer makes sense to redo the `add n/David ...` command. This is the behavior that most modern desktop applications follow.
		How the redo feature works is very similar to how to undo feature work. The difference is that redo utilises the undoneCommands stack and the undoneAddressBook stack. As such, you can also refer to the diagrams in the Undo Implementation.

Update UG/DG and undo for training #188

Update UG/DG and undo for training #188

Conversation

junhuplim commented Nov 2, 2019

coveralls commented Nov 2, 2019 • edited

Pull Request Test Coverage Report for Build 259

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellodommy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 2, 2019 •

edited