Skip to content

[CS2113-T18-1] Movie Reviews#47

Open
naz019 wants to merge 445 commits into
nus-cs2113-AY2223S1:masterfrom
AY2223S1-CS2113-T18-1b:master
Open

[CS2113-T18-1] Movie Reviews#47
naz019 wants to merge 445 commits into
nus-cs2113-AY2223S1:masterfrom
AY2223S1-CS2113-T18-1b:master

Conversation

@naz019

@naz019 naz019 commented Oct 18, 2022

Copy link
Copy Markdown

No description provided.

@nus-se-bot

Copy link
Copy Markdown

@naz019 Closing this PR because this PR does not seem to comply with our requirements. Possible reasons:

  1. You sent this PR to the wrong repo. Only PRs from your team repo should be sent here.
  2. This PR is not sent from the master branch of your team repo.
  3. Your team org is named incorrectly. It should be AY2223S1-CS2113-T18-1
    If that is the case, you can reopen this PR after renaming the org with the correct name.

@nus-se-bot nus-se-bot closed this Oct 20, 2022
@okkhoy okkhoy reopened this Oct 26, 2022
Comment thread docs/DeveloperGuide.md Outdated
@@ -2,7 +2,7 @@

## Acknowledgements

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the Ui UML Diagram, the accessibility of the attributes and methods are represented by different symbols (such as diamonds, square etc). Maybe you might want to disable the skinparam to display as '+' and '-' as how we usually do those?

Comment thread docs/DeveloperGuide.md

## Acknowledgements

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Media class is shown as abstract in the UML diagram. However, it is not implemented as an abstract class. Is it supposed to be just shown as 'normal' class in the UML diagram?

Comment thread docs/DeveloperGuide.md Outdated
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
AddressBook-Level3 https://github.com/se-edu/addressbook-level3

## Design & implementation

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Screenshot 2022-10-26 at 12 27 37 PM

Comment thread docs/DeveloperGuide.md Outdated
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
AddressBook-Level3 https://github.com/se-edu/addressbook-level3

## Design & implementation

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Screenshot 2022-10-26 at 12 40 03 PM

@hughjazzman hughjazzman 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.

Some comments! Some bugs are repeated, so make sure to fix them for all diagrams.

Comment thread docs/DeveloperGuide.md
* Output various messages to the user

## Design & implementation
![img.png](imgs/UiClass.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.

Note that this is non-standard UML notation. You can use the info here to change settings for PlantUML: nus-cs2113-AY2122S1/forum#114

image

Comment thread docs/DeveloperGuide.md
The command component enables users to make changes to their review list. The command word is taken from the first word
of the user input, and is processed through the `Parser` class. The class diagram shows how the Commands parent class is
implemented, as well as its extended classes.
![img.png](imgs/CommandsClass.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

Might this italicised String execute be a bug?

Comment thread docs/DeveloperGuide.md
DukeException error is thrown.
4. A new variable of type Media parent class, `toAdd`, is then created. The `AddCommand` function is then called to add
the Movie or Tv Show `toAdd` into the review list, `reviewList`.
![img_1.png](imgs/AddCommandSequence.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.

Consider adding : to signify these as instance of their classes.

image

Comment thread docs/DeveloperGuide.md
4. A new variable of type Media parent class, `toAdd`, is then created. The `AddCommand` function is then called to add
the Movie or Tv Show `toAdd` into the review list, `reviewList`.
![img_1.png](imgs/AddCommandSequence.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.

Consider breaking up the diagram. If showing the sequence of AddComand, you can concentrate on the sequence that is unique to AddCommand. Reference frames could be useful.

image

Comment thread docs/DeveloperGuide.md
4. A new variable of type Media parent class, `toAdd`, is then created. The `AddCommand` function is then called to add
the Movie or Tv Show `toAdd` into the review list, `reviewList`.
![img_1.png](imgs/AddCommandSequence.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.

There may be missing/extra lifelines here.

image

image

Comment thread docs/DeveloperGuide.md Outdated
2. The `Parser` object calls on `executeList` which is responsible for creating a `ListCommand` Object. It then calls on its `execute` method to execute the proper actions.
3. In this `execute` function, it loops through all the stored reviews and separates each review based on category and formats using the proper `toString` method.
4. Finally, an output string is returned and the `Parser` class calls on the `UI` object to print the output to the user.
![img_2.png](imgs/ListCommandSequence.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.

This could be a bug, is this a self-invocation?

image

Comment thread docs/DeveloperGuide.md Outdated
2. The `Parser` object calls on `executeList` which is responsible for creating a `ListCommand` Object. It then calls on its `execute` method to execute the proper actions.
3. In this `execute` function, it loops through all the stored reviews and separates each review based on category and formats using the proper `toString` method.
4. Finally, an output string is returned and the `Parser` class calls on the `UI` object to print the output to the user.
![img_2.png](imgs/ListCommandSequence.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.

This return might need to be dotted?

image

indraneelrp and others added 30 commits November 7, 2022 19:49
updated favourites command
updated UG with latest output
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.

8 participants