Skip to content

[CS2113-T12-2] Big-PP#23

Open
Magmanat wants to merge 615 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T12-2:master
Open

[CS2113-T12-2] Big-PP#23
Magmanat wants to merge 615 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T12-2:master

Conversation

@Magmanat

@Magmanat Magmanat commented Mar 2, 2023

Copy link
Copy Markdown

This is a command-line tool designed to help you pick computer parts based on your needs and budget.

kyrixn added a commit to kyrixn/tp that referenced this pull request Mar 13, 2023
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated

@SpawnageLoong SpawnageLoong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall good explanation, but class diagrams tend to be excessively detailed to the point that they cannot be read without zooming in.

Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md

@kairuler kairuler left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall really nice DG :) I liked how some of the classes use a Initialization, Functionality, Behavior structure

Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
![Viewer Add Command Sequential Diagram](uml-pictures/ViewerAddCommand.png)
### PC Class Architecture

![PC Class Diagram](uml-pictures/PcDiagram.png)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image
image

In the DG, the Component class is said to be an abstract class but in the diagram, it isn't shown as an abstract class. If I didn't misunderstand the diagram, perhaps this is a mistake?

Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/DeveloperGuide.md
## Design & implementation

{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
### UI class

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe a multiplicity needs to be added between UI and UIState as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.