[AB3: Tracing Code] Add Model tracing code

[UdhayaShan1]

Issue mentioned in #16

While tutorial touches on the Logic overview, I believe it can be useful to touch on the Model interactions as well. There could be existing confusion on FilteredPersons and hence this could alleviate some doubts, especially if they wish to work with AddressBook's list of persons.

Using the same edit command,

1. New sequence diagram to show Model lower level

2. Continue with tracing similar to tutorial
3. Break up the code into sections so they better understand the purpose of the code

[damithc]

@UdhayaShan1 I can see how this can provide more explanations to the reader. What I don't like is that the UML diagram seemingly show low-level details of two components. This is caused by the filtered list, which is used for data exchange, is appearing inside the Model component. Let me fiddle with this diagram a bit and see if there's a way around it.

BTW, no need to commit the generated UML diagram. MarkBind generates it automatically. Just put the .puml file where you put the .png file, and use a <puml ... tag in the code where you need to show the diagram. To give an example from the same codebase:

<puml src="images/add-remark/ParserClass.puml" 
    alt="The relationship between Parser and RemarkCommandParser"/>

[NeoHW]

Insightful changes! I have some comments regarding some nits and best practices which can be considered ◡̈

[NeoHW]

Perhaps could consider using generic numbers (1. for all instead of 2., 3....)
With generic numbers, we would be able to insert items into the middle of the list without modifying others in the future.

[UdhayaShan1]

Excellent thanks

[JiaXinEu]

I like how this provides a better understanding of the interaction between command and model.

[JiaXinEu]

Do you think the arrows for method call should be solid lines instead of dashed lines (since those indicate returns)
eg.

EditCommand -> Model : setPerson(person, editedPerson)

Additionaklly, perhaps the call to setPredicate can be ommitted?

[UdhayaShan1]

1. Yep your right, was a mistake to put dotted arrow

2. Yes seems unnecessary, I will remove that call

Thanks!

[damithc]

Suggested:

Changes:

1. Moved FilteredPersonList out of Model. Although the class is in the Model, and the object is kept as a variable inside ModelManager, it simply acts as a data transfer object between the two components. So, better to show outside of both components. This also solves the problem of more than one component showing low-level details (as the Model now does not show low level details).
2. Added object names, semicolon before class name
3. Used solid arrow for method calls
4. Show parameters and return values where useful
5. Removed setPredicate(...) as that's an internal detail of the Model.

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

[UdhayaShan1]

Thanks Prof, will update existing puml with this and work on it as needed.

[UdhayaShan1]

I think I will omit the arguments as it can referred in the code and will not come out too overwhelming

[UdhayaShan1]

Updated according to @NeoHW ,@JiaXinEu and @damithc comments

Thanks!

[bennyLCK]

The generation of a CommandResult object could possibly also be labelled with CommandResult( ) to signify that we are calling its constructor. a "Cross" could be added after CommandResult returns in the diagram to signify that it is later on dereferenced and is left for garbage collection.

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.

[UdhayaShan1]

Thanks for the suggestion but I think its best to leave out the deletion here as CommandResult wont be deleted until far later until MainWindow utilises its getFeedback()

Appreciate the feedback

[bennyLCK]

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.