-
Notifications
You must be signed in to change notification settings - Fork 26
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
[AB3: Tracing Code] Add Model
tracing code
#45
base: master
Are you sure you want to change the base?
Changes from 5 commits
07e8048
14512d0
d8b9a8b
48815bb
1aa9392
a6fbba0
8224fe7
ed88ddb
47e2cab
d516315
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -273,6 +273,91 @@ Recall from the User Guide that the `edit` command has the format: `edit INDEX [ | |
{{ dg_ref }} This is a good time to read through the [**_UI component_** section of the DG](https://se-education.org/addressbook-level3/DeveloperGuide.html#ui-component) | ||
|
||
|
||
## Misc: Tracing `Model` execution path | ||
|
||
1. In our previous discussions, we've primarily focused on the `Logic` component to understand the general flow of logic when executing commands. | ||
2. Now, let's delve into how the `EditCommand#execute()` method interacts with the `Model` component. | ||
3. Let us reproduce the full code of `EditCommand#execute()` | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
**`EditCommand#execute()`:** | ||
```java | ||
@Override | ||
public CommandResult execute(Model model) throws CommandException { | ||
requireNonNull(model); | ||
List<Person> lastShownList = model.getFilteredPersonList(); | ||
|
||
if (index.getZeroBased() >= lastShownList.size()) { | ||
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); | ||
} | ||
|
||
Person personToEdit = lastShownList.get(index.getZeroBased()); | ||
Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor); | ||
|
||
if (!personToEdit.isSamePerson(editedPerson) && model.hasPerson(editedPerson)) { | ||
throw new CommandException(MESSAGE_DUPLICATE_PERSON); | ||
} | ||
|
||
model.setPerson(personToEdit, editedPerson); | ||
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); | ||
return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, Messages.format(editedPerson))); | ||
} | ||
``` | ||
4. We hope that you have developed an appreciation for sequence diagrams in understanding high-level overviews of AB3. | ||
Below is a sequence diagram, with some method calls omitted for brevity, illustrating the interactions within the Model component during the execution of an edit command. | ||
<puml src="images/tracing/EditSequenceDiagramModelHighLevel.puml" | ||
alt="Tracing an `edit` command through the Model component"/> | ||
5. Let us put a breakpoint at this line which will be the first passing of control to `Model` in `EditCommand#execute()`. | ||
![ModelBreakpoint](images/tracing/ModelBreakpoint.png) | ||
6. Stepping into the Model component reveals that it returns an `FilteredList<Person>`. | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### What is FilteredList<Person>? | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
```java | ||
public ModelManager(ReadOnlyAddressBook addressBook, ReadOnlyUserPrefs userPrefs) { | ||
requireAllNonNull(addressBook, userPrefs); | ||
|
||
logger.fine("Initializing with address book: " + addressBook + " and user prefs " + userPrefs); | ||
|
||
this.addressBook = new AddressBook(addressBook); | ||
this.userPrefs = new UserPrefs(userPrefs); | ||
filteredPersons = new FilteredList<>(this.addressBook.getPersonList()); | ||
} | ||
``` | ||
|
||
1. Peeking into the constructor reveals `filteredPersons` is created with a `FilteredList` wrapped around | ||
Addressbook's internal list. | ||
2. `FilteredList` is a wrapper around an existing list and, as the name suggests, applies a filter to determine which | ||
elements from the original list should be included. | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
3. Any modifications to the original list (e.g., adding, removing, or updating elements) will be visible in the FilteredList. | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
### Model's AddressBook | ||
|
||
|
||
1. Continuing on, let us step over until the next `Model` interaction, `model.hasPerson(editedPerson)`. | ||
|
||
<img src="images/tracing/modelhasPerson.png" alt="setPredicate" width="600"> | ||
|
||
2. This reveals our first pass of control to `Addressbook` | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
2. Upon further inspection, the `Addressbook` is merely invoking the contains method of its internal list of persons. | ||
[Remember](#what-is-filteredlist), this is the same list that the `ModelManager`'s `FilteredList` wraps around. | ||
3. Stepping over again brings us to `model.setPerson` which again further calls on `Addressbook` to `setPerson` as it has access to its internal list. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. number seems off, generic numbering might help |
||
|
||
### Filtering the FilteredPersons | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
1. Now we should be at `model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS)`. | ||
<img src="images/tracing/setPredicate.png" alt="setPredicate" width="800"> | ||
2. Stepping into this method, we see it calls `filteredPersons.setPredicate(predicate)`. | ||
3. Predicates enable filtering the list based on specific criteria. _Hint: Another feature uses this too!_ | ||
4. For `EditCommand`, `PREDICATE_SHOW_ALL_PERSONS` is used to clear any existing filters, | ||
ensuring that all persons are displayed. | ||
5. A `CommandResult` is returned with a message displaying edited `Person`. | ||
UdhayaShan1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
<box type="tip" seamless> | ||
How can you use what you learnt to develop a sort feature? | ||
</box> | ||
|
||
## Conclusion | ||
|
||
In this tutorial, we traced a valid edit command from raw user input to the result being displayed to the user. From this tutorial, you learned more about how the various components work together to produce a response to a user command. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The generation of a All in all, good job generating this sequence diagram and I hope my comments find you well and lets remember these annoying nitty gritty details as they would make UML diagrams much easier for us to generate and understand in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion but I think its best to leave out the deletion here as Appreciate the feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem, and it's always good to clarify when an object is "destroyed" especially to new students who are unfamiliar with the concept. Thanks for your kind response. |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think the arrows for method call should be solid lines instead of dashed lines (since those indicate returns)
Additionaklly, perhaps the call to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
@startuml | ||
!include ../style.puml | ||
skinparam ArrowFontStyle plain | ||
|
||
box Logic LOGIC_COLOR_T1 | ||
participant "EditCommand" as EditCommand LOGIC_COLOR | ||
participant "CommandResult" as CommandResult LOGIC_COLOR | ||
end box | ||
|
||
box Model MODEL_COLOR_T1 | ||
participant "FilteredList<Person>" as ObservableList MODEL_COLOR | ||
participant "ModelManager" as Model MODEL_COLOR | ||
end box | ||
|
||
--> EditCommand | ||
activate EditCommand | ||
|
||
|
||
|
||
EditCommand -> Model : getFilteredPersonList() | ||
activate Model | ||
Model --> EditCommand | ||
deactivate Model | ||
|
||
|
||
EditCommand --> ObservableList : retrieve Person | ||
activate ObservableList | ||
ObservableList --> EditCommand : Person | ||
deactivate ObservableList | ||
|
||
EditCommand --> EditCommand : createEditedPerson | ||
activate EditCommand | ||
EditCommand --> EditCommand | ||
deactivate EditCommand | ||
|
||
EditCommand --> Model : setPerson | ||
activate Model | ||
Model --> EditCommand | ||
deactivate Model | ||
|
||
EditCommand --> Model : updateFilteredPersonList | ||
activate Model | ||
Model --> ObservableList : setPredicate | ||
activate ObservableList | ||
ObservableList --> Model | ||
deactivate ObservableList | ||
Model --> EditCommand | ||
deactivate Model | ||
|
||
|
||
|
||
|
||
create CommandResult | ||
EditCommand --> CommandResult | ||
activate CommandResult | ||
CommandResult --> EditCommand | ||
deactivate CommandResult | ||
|
||
[<-- EditCommand | ||
deactivate EditCommand | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes:
You can tweak further, as needed. @startuml
!include ../style.puml
skinparam ArrowFontStyle plain
box Logic LOGIC_COLOR_T1
participant ":EditCommand" as EditCommand LOGIC_COLOR
participant "r:CommandResult" as CommandResult LOGIC_COLOR
end box
participant "filteredPersons:FilteredList<Person>" as ObservableList MODEL_COLOR
box Model MODEL_COLOR_T1
participant "model:ModelManager" as Model MODEL_COLOR
end box
-> EditCommand : execute(model)
activate EditCommand
EditCommand -> Model : getFilteredPersonList()
activate Model
Model --> EditCommand : filteredPersons
deactivate Model
EditCommand -> ObservableList : get(index)
activate ObservableList
ObservableList --> EditCommand : personToEdit
deactivate ObservableList
EditCommand -> EditCommand : createEditedPerson(personToEdit, editPersonDescriptor)
activate EditCommand
EditCommand --> EditCommand: editedPerson
deactivate EditCommand
EditCommand -> EditCommand : check for duplicates
activate EditCommand
deactivate EditCommand
EditCommand -> Model : setPerson(personToEdit, editedPerson)
activate Model
Model --> EditCommand
deactivate Model
EditCommand -> Model : updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS)
activate Model
deactivate ObservableList
Model --> EditCommand
deactivate Model
create CommandResult
EditCommand -> CommandResult
activate CommandResult
CommandResult --> EditCommand
deactivate CommandResult
[<-- EditCommand: r
deactivate EditCommand
@enduml
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Prof, will update existing puml with this and work on it as needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I will omit the arguments as it can referred in the code and will not come out too overwhelming |
||
@enduml |
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.
Perhaps could consider using generic numbers (
1.
for all instead of2.
,3.
...)With generic numbers, we would be able to insert items into the middle of the list without modifying others in the future.
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.
Excellent thanks