Skip to content
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

DeveloperGuide.md: update find, clear, list person #187

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions docs/DeveloperGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,24 +250,77 @@ The activity diagram shows the general sequence of steps when a user interacts w

#### Implementation

1. **Input Parsing**
The user command input is parsed in `LogicManager` by calling `AddressBookParser#parseCommand`. For a command starting with "find person", the parser creates a `FindPersonCommandParser` instance to parse specific parameters.
Comment on lines +253 to +254
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this input parsing part for each feature, since it would be covered under the general implementation of the command at the start of the section? (you can refer to #186 for the general implementation of the entity commands, currently being worked on)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I see that I may have to modify and remove redundant steps in implementation and design consideration. I will make appropriate changes as soon as we reach a consensus on the overall structure of our Feature part of DG.


2. **Argument Tokenization**
`FindPersonCommandParser#parse` uses `ArgumentTokenizer` to separate user input into arguments prefixed by indicators (e.g., `n/` for names).

3. **Predicate Creation**
The `FindPersonCommandParser` checks if the `n/` prefix is present. If so, it creates a `NameContainsKeywordsPredicate` instance, containing a list of keywords derived from the user input, and passes it to `FindPersonCommand`.

4. **Command Execution**
Upon execution, `FindPersonCommand#execute` calls `Model#updateFilteredPersonList` to apply the predicate, filtering the list of persons based on keyword matches. The command then constructs a success message indicating the number of persons found.

5. **Error Handling**
- If the required prefix (e.g., `n/`) is missing, the parser throws a `ParseException`.
- If an `IOException` or `AccessDeniedException` occurs during data storage, `LogicManager` handles these with an appropriate `CommandException`.

#### Design considerations

### 1. Predicate Composition
- **Reason**: Using predicates enables flexible filtering and reduces redundancy by allowing shared filtering logic across commands.

### 2. Case-Insensitive Matching
- **Reason**: User experience is enhanced by making searches case-insensitive.

Comment on lines +271 to +276
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good!


<br>

### List person feature

#### Implementation

1. **Input Parsing**
The `list` command input is parsed in `LogicManager` through the `AddressBookParser#parseCommand` method. When the command starts with `list`, the parser invokes `ListCommandParser` to handle the specific arguments.

2. **Entity Type Verification**
`ListCommandParser#parse` trims the input to verify the entity type, checking for `PERSON_ENTITY_STRING` or `APPOINTMENT_ENTITY_STRING`.
If the entity type is `PERSON_ENTITY_STRING`, `ListCommandParser` returns an instance of `ListPersonCommand`.
Comment on lines +287 to +289
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clear, but should we include it at the start of the section, since it applies to all the commmandparsers?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too.


3. **Command Execution**
When executed, `ListPersonCommand#execute` calls `Model#updateFilteredPersonList` with the predicate `PREDICATE_SHOW_ALL_PERSONS` to display all persons.

4. After updating the list, the `ListPersonCommand` returns a success message indicating that all persons have been listed.

#### Design considerations

### 1. Abstracting Common List Logic
- **Reason**: `ListCommand` is designed as an abstract class to handle shared listing logic for different entities (persons and appointments).

<br>

### Clear person feature

#### Implementation

1. **Input Parsing**
The `clear` command input is parsed in `LogicManager` by calling `AddressBookParser#parseCommand`. When the command starts with clear, the parser invokes `ClearCommandParser` to process specific arguments.

2. **Entity Type Verification**
`ClearCommandParser#parse` checks if the trimmed input matches `PERSON_ENTITY_STRING` or `APPOINTMENT_ENTITY_STRING`.
If the entity type is `PERSON_ENTITY_STRING`, `ClearCommandParser` returns an instance of `ClearPersonCommand`.

3. **Command Execution**
When executed, `ClearPersonCommand#execute` calls `Model#setAddressBook` with a new, empty AddressBook to clear all person entries.
After clearing, `ClearPersonCommand` returns a success message indicating the address book has been cleared.

#### Design considerations

### 1. Abstracting Clear Logic
- **Reason**: The `ClearCommand` abstract class handles shared logic for clearing different entities (person or appointment).
Comment on lines +320 to +321
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this to the start of the section, since the abstraction applies for all the commands?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this abstract logic is common throughout all commands, we should consider making a general design consideration section.



<br>

### Add appointment feature
Expand Down
15 changes: 15 additions & 0 deletions docs/team/choiwab.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,18 @@
---

Given below are my contributions to the project.

Role:
- Developer
- Storage
- Code Quality

Given below are my contributions to the project:
- Refactoring of `EditCommand` Command to support new command format
- Creation of `EditAppointmentCommand` command
- Adding storage classes associated with appointment
- Create `clearAppointmentCommand` feature
- Adding test cases for `clearAppointmentCommand` feature
- Update UI (Labels, date formatting, minor bug fixes)
- Overall code quality check