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

[CS2113-T14-2] eCardnomics #39

Open
wants to merge 541 commits into
base: master
Choose a base branch
from

Conversation

kaijiel24
Copy link

@kaijiel24 kaijiel24 commented Oct 1, 2020

Flash Card manager for Economics students on Command Line


#### Commands

![DG-Design Commands UML](./images-dg/DG-Design-Commands.png?raw=true "Commands UML Class Diagram")

Choose a reason for hiding this comment

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

Maybe you would like to break this diagram into two parts, so that it is less complicated?

Choose a reason for hiding this comment

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

Changed, thanks!

interacts with `GameStorage` on package-private basis; i.e., `GameEngine` and `GameStorage` have full mutual
access as if they were a single class. This is one of the main intentional design decisions.

![DG-Implementation-Features-Game-Mode-Architecture](./images-dg/Game-Mode-Design.png?raw=true "Game Mode

Choose a reason for hiding this comment

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

Maybe you would like to split up this digram into two parts, so that it is less complicated?

Choose a reason for hiding this comment

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

Changed, thanks!


The following elaborates the execution flow of Game Mode, from after a `start` command has been parsed in Normal Mode:

![DG-Implementation-Features-Game-Mode-Sequence](./images-dg/Game-Mode-Sequence.png?raw=true "Game Mode UML Sequence

Choose a reason for hiding this comment

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

Maybe it would be better to simplify this diagram/ split them up into smaller parts, so that it would be easier to follow?

Choose a reason for hiding this comment

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

Changed with reference frames, thanks!

An additional feature targeted at students who wish to use add more style to their flash cards outside of the command
line option to allow keep things interesting when they are revising.

The `PowerPointCommand` is parsed by `NormalParser` but the "Print to PowerPoint" command can be called from both Normal

Choose a reason for hiding this comment

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

Should there be a return to PowerpointCommand after Powerpoint is created in the sequence diagram?

I believe that the box to contain the "opt" is wrong, you may refer to the notes again, it should be a sharp box with a chip of the corner.

Copy link
Author

Choose a reason for hiding this comment

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


#### How to **components** interact with one another
![Sequence Diagram](images-dg/Sequence%20Diagram.png)
The **Sequence Diagram** above shows how the components interact for a basic `create <deck name>` command where a new deck is created and added in to the `Deck List`.

Choose a reason for hiding this comment

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

image
I believe the return arrow should be a dotted line

The arrows coming into the UI class shouldnt be into or from the middle of the box, should be from the top or bottom of the box

Copy link
Author

Choose a reason for hiding this comment

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

The user can also modify to tags of the decks by using tag or untag command, and uses search by tag to find
a group of decks he/she is interested in.

![DG-Implementation-Features-TagSequence](./images-dg/Tag.png?raw=true)

Choose a reason for hiding this comment

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

image
Should there be a return arrow? Prof said that it is optional if a void function is called

Copy link

@siewwencode siewwencode left a comment

Choose a reason for hiding this comment

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

Overall the DG diagrams looks engaging in a sense that the diagrams are colored. The text explanation is clear and easy to understand.
However, you can consider having different markdowns for class and methods.

* `Deck List` : A complete list of all the `Deck`s in memory.
* `Storage` Reads and writes data from and to a text file.

#### How to **components** interact with one another

Choose a reason for hiding this comment

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

This part is a bit confusing? Perhaps you can elaborate more?

image

Copy link
Author

Choose a reason for hiding this comment

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

* `Storage` Reads and writes data from and to a text file.

#### How to **components** interact with one another
![Sequence Diagram](images-dg/Sequence%20Diagram.png)

Choose a reason for hiding this comment

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

You can consider resizing your diagram such that the text size in the diagram matches the the text size of the main text of the diagram.

image

Copy link
Author

Choose a reason for hiding this comment

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

* `Storage` Reads and writes data from and to a text file.

#### How to **components** interact with one another
![Sequence Diagram](images-dg/Sequence%20Diagram.png)

Choose a reason for hiding this comment

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

Should this be a dotted arrow?

image

Comment on lines 48 to 50
1. The overall logic component consists of the Parser class and Command class.
2. The Parser parses the user input and creates the respective Command object.
3. This command will be executed by the Main class.

Choose a reason for hiding this comment

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

You might have missed out bolding the classes here

sequence diagram showcasing this interaction, for execution of a `CreateCommand`, e.g. `create
microeconomics`:

![DG-Design CreateCommand Sequence UML](./images-dg/DG-Design-Sequence-Diagram.png?raw=true "CreateCommand UML

Choose a reason for hiding this comment

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

You can consider terminating CreateCommand

image

Copy link
Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Won't terminate CreateCommand here because this just shows the creation of CreateCommand, returning createCommand object whose sole purpose of life is to have its execute() method called, but in this sequence diagram that is essentially a snippet of the lifeline of createCommand I cannot also include its interactions with Main class because that's out of scope here; the point is that createCommand does exist for the whole duration shown here.

Although, I should include some textual description of the gist of what I am conveying here, thanks!

and Deck Mode.

The following diagram shows how the `PowerPointCommand`'s `execute()` calls the `createNewPowerPoint()` method of `PowerPoint`.
![PPTX Sequence Diagram](images-dg/DG-PPTX-Sequence-Diagram.png)

Choose a reason for hiding this comment

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

Are you missing the return arrows?

image

`toString(boolean isQuestion, int offset)` method of a `FlashCard`
object.

![DG-Implementation-Features-PP-Sequence](./images-dg/PP-Sequence.png?raw=true)

Choose a reason for hiding this comment

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

Are you labelling correct? Perhaps it should be toString(isQuestion, offset) and formatResponse(isQuestion, offset)

image

Copy link
Author

Choose a reason for hiding this comment

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

The user can also modify to tags of the decks by using tag or untag command, and uses search by tag to find
a group of decks he/she is interested in.

![DG-Implementation-Features-TagSequence](./images-dg/Tag.png?raw=true)

Choose a reason for hiding this comment

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

Are you missing a return arrow here?

image

Choose a reason for hiding this comment

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

A minor comment: For the sequence diagram below, would it be better if Main class is put on the left-most side? Because I misunderstood it first.
image

I believe the idea we were trying to convey is that the user does not interact directly with the Main class, but directly with the Ui class instead via the Ui's readUserInput() method.


The following elaborates the execution flow of Game Mode, from after a `start` command has been parsed in Normal Mode:

![DG-Implementation-Features-Game-Mode-Sequence](./images-dg/Game-Mode-Sequence.png?raw=true "Game Mode UML Sequence

Choose a reason for hiding this comment

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

some return arrows are missing

image

Choose a reason for hiding this comment

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

Thanks for the input! Return arrows are not mandatory, so this was intentional.

Copy link

@samuellleow samuellleow left a comment

Choose a reason for hiding this comment

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

Overall great design and explanation of the DG, do consider and review the mistakes I pointed.

The user can also modify to tags of the decks by using tag or untag command, and uses search by tag to find
a group of decks he/she is interested in.

![DG-Implementation-Features-TagSequence](./images-dg/Tag.png?raw=true)

Choose a reason for hiding this comment

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

image

I believe that you can terminate the Ui class after printing the required statement

Copy link
Author

Choose a reason for hiding this comment

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

alwaysnacy and others added 30 commits November 9, 2020 22:11
…into master

# Conflicts:
#	docs/DeveloperGuide.md
#	docs/images-dg/Tag-feature.png
Update DG Storage sequence diagram to have transparent background
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.