-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor class names #98
Refactor class names #98
Conversation
Codecov Report
@@ Coverage Diff @@
## master #98 +/- ##
=========================================
Coverage 75.95% 75.95%
Complexity 719 719
=========================================
Files 109 109
Lines 2071 2071
Branches 225 225
=========================================
Hits 1573 1573
Misses 444 444
Partials 54 54
Continue to review full report at Codecov.
|
…-names # Conflicts: # src/main/java/atas/model/attendance/Session.java
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.
LGTM! Just some nits.
docs/DeveloperGuide.md
Outdated
@@ -25,7 +25,7 @@ title: Developer Guide | |||
* [Glossary](#glossary) | |||
* [**Appendix: Instructions for manual testing**](#appendix_manual_testing) | |||
* [Launch and shutdown](#launch_shutdown) | |||
* [Deleting a person](#deleting_a_person) | |||
* [Deleting a student](#deleting_a_studentn) |
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.
Should "studentn" be "student instead?
@@ -5,7 +5,7 @@ title: "Tutorial: Adding a command" | |||
|
|||
Let's walk you through the implementation of a new command — `remark`. | |||
|
|||
This command allows users of the AddressBook application to add optional remarks to people in their address book and edit it if required. The command should have the following format: | |||
This command allows users of the StudentList application to add optional remarks to people in their student list and edit it if required. The command should have the following format: |
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.
Should people be "student' instead?
docs/tutorials/AddRemark.md
Outdated
model.setPerson(personToEdit, editedPerson); | ||
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); | ||
model.setStudent(studentToEdit, editedStudent); | ||
model.updateFilteredStudentList(PREDICATE_SHOW_ALL_PERSONS); |
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.
Should the constant messages be renamed as well? I've noticed this issue in other places as well.
…ass-names # Conflicts: # src/test/java/atas/logic/commands/sessionlist/EditSessionCommandTest.java # src/test/java/atas/logic/commands/sessionlist/session/ParticipateCommandTest.java # src/test/java/atas/logic/commands/sessionlist/session/PresenceCommandTest.java # src/test/java/atas/logic/commands/studentlist/DeleteSessionCommandTest.java # src/test/java/atas/logic/parser/AtasParserTest.java # src/test/java/atas/logic/parser/ParserUtilTest.java
@@ -19,7 +19,7 @@ activate model MODEL_COLOR | |||
model -[MODEL_COLOR]-> logic | |||
deactivate model | |||
|
|||
logic -[LOGIC_COLOR]> storage : saveAddressBook(addressBook) | |||
logic -[LOGIC_COLOR]> storage : saveAddressBook(studentList) |
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.
Should this be something like saveStudentList
?
@@ -180,7 +180,7 @@ Recall from the User Guide that the `edit` command has the format: `edit INDEX [ | |||
* {@code JsonSerializableAddressBook}. | |||
*/ | |||
public JsonSerializableAddressBook(ReadOnlyAddressBook source) { |
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 change this to JsonSerializableStudentList
and correspondingly ReadOnlyStudentList
?
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.
LGTM!
Refactor class names