nus-cs2103-AY2122S1 / pe-dev-response

0 stars 0 forks source link

Weird dependancy on ModelManager and CCA #857

Open nus-pe-bot opened 2 years ago

nus-pe-bot commented 2 years ago

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.


Based from your diagram below of your model component,

AddressBook has a uniqueCCAList and the list access the CCA. But why is your ModelManager accessing your CCA class? It seems your ModelManager is jumping to somewhere it should not be.

Screenshot 2021-11-12 at 5.19.50 PM.png


[original: nus-cs2103-AY2122S1/pe-interim#892] [original labels: severity.Medium type.DocumentationBug]

jovyntls commented 2 years ago

Team's Response

Thanks for your suggestion.

ModelManager does indeed access the Cca class. Hence, our UML diagram here is correct.

As to why ModelManager accesses the Cca class even though it also accesses the UniqueCcaList: please refer to the code snippet below, taken from our ModelManager.java.

    @Override
    public boolean hasCca(Cca cca) {
        requireNonNull(cca);
        return addressBook.hasCca(cca);
    }

    @Override
    public void deleteCca(Cca target) {
        addressBook.removeCca(target);
    }

    @Override
    public void addCca(Cca cca) {
        addressBook.addCca(cca);
        updateFilteredCcaList(PREDICATE_SHOW_ALL_CCAS);
    }

    @Override
    public void setCca(Cca target, Cca editedCca) {
        requireAllNonNull(target, editedCca);
        addressBook.setCca(target, editedCca);
    }

Here, ModelManager has to access the CCA class. We believe the UML diagram is correct, and the architecture decision is fully justifiable and does not break OOP principles.

Duplicate status (if any):

--