[CS2113T-F12-2] AniChan#54
Conversation
BenardoTang
left a comment
BenardoTang
left a comment
There was a problem hiding this comment.
Very well documented and explained for the most part. However, do take a look at some of the diagrams as they seem overly complicated and some don't load in the website. Good Job!!
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
The sequence diagram dosen't seem to load in the DG. Seems to happen for other components below...
| <br/> | ||
|
|
||
| ### 4.1 Estimation Feature | ||
|
|
There was a problem hiding this comment.
This portion seems overly confusing to a new user like me... i think a flow diagram/class diagram would greatly help with explaining this implementation.
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
The diagram is too complicated, do consider splitting the diagram up to smaller portions. It is also difficult to read the words inside the diagram
| <br/> | ||
|
|
||
| ##### Aspect: When should the program read the script file | ||
|
|
There was a problem hiding this comment.
While i feel the explanations here are well done, It could be too wordy and overly complicated. It should be easier to understand with a general flow diagram to justify your design choices.
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Should the Sequence diagram be split into different portions? It is difficult to understand what's going on...
| An example scenario would be browsing the 2nd page of a **sorted** list in ascending order. | ||
| The only step that would change would be at Step 3, where it will perform sorting of `AnimeData` list. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
I like that there is a diagram for almost after every step, well done!
Bryanbeh1998
left a comment
Bryanbeh1998
left a comment
There was a problem hiding this comment.
Overall a very well documented DG! Would look forward to trying out your product. Perhaps you can work on making the sequence diagram less complex. There is a part where it may seem a little too wordy as well. Great job!
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Diagram is too complex. The words may also be a little too small for people to read.
There was a problem hiding this comment.
Perhaps you can consider breaking it down further
|
|
||
| The user executes `watchlist -d 2` to delete the second watchlist ("NewAnime") in the list. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Words are too small in the sequence diagram. Hard for the user to read and follow
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
The diagram does not load on the website. Perhaps you can check again
|
|
||
| <br/> | ||
|
|
||
| #### 4.1.2 Design Consideration |
There was a problem hiding this comment.
This section although well documented, may seem a little too wordy
skyaxe97
left a comment
skyaxe97
left a comment
There was a problem hiding this comment.
Looks good and detailed! Do look out for when the fonts in the diagrams get a little too tiny. Besides that, good work!
| ## 1. Introduction | ||
| **AniChan** is a command-line application written in **Java 11**. It is written using the Object-Oriented Programming (OOP) paradigm which provides us with means to structure a software program into organized, reusable and reusable pieces of code that makes it good for future improvements and revisions. | ||
| <br/> |
There was a problem hiding this comment.
Should a more detailed introduction of the program be included here to introduce the functionality of the application?
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Is there a missing name of method call from XYZParser to Command? And also, would it be better if the line from the activation bar of XYZParser to Command be lower on the activation bar? (It doesn't appear too distinct on the website)
|
|
||
| <br/> | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Maybe consider using reference frames to split the diagram into different portions to make it more readable? It may consequently help to increase the font size of the wordings as it does get a little too small to read.
| `BrowseCommand#buildBrowseOutput()` will shift its page window down by 1 page as depicted in the diagram below. | ||
|
|
||
|  <br/> | ||
| *Figure 13: Browse Next Page Object Diagram* |
There was a problem hiding this comment.
Good use of object diagrams to reflect the states.
|
Hi team F12-2, please rename your PR with CS2113T instead. |
chewyang
left a comment
chewyang
left a comment
There was a problem hiding this comment.
DG is well formatted and consistent, looks neat and diagrams are accurate and used when needed. Although some UML diagrams could be cropped and zoomed in to the use, omitting the main and parser classes as it may be a little repetitive. But overall great effort!
|
|
||
| <br/> | ||
|
|
||
| ### 3.3 Parser Component |
There was a problem hiding this comment.
Hi! this is a very clear diagram but should this arrow be a arrow with an empty head instead to show inheritance?
| *Figure 21: Bookmark Entries with more Add* | ||
|
|
||
| The following sequence diagram shows how the `Add Bookmark` operation works: | ||
|
|
There was a problem hiding this comment.
Great use of colours to show the components, however should this diagram include sequences already introduced earlier? will showing the sequence after the execute() be clearer?
|
|
||
|  <br/> | ||
| *Figure 23: Bookmark Entries After Delete* | ||
|
|
There was a problem hiding this comment.
Great use of tables to illustrate the bookmarking implementation!
| Though the 1st approach could have created a more authentic browsing feature it would not fit the requirements as well as the | ||
| 2nd approach. The 2nd approach allows for more precise browsing of pages means that more experienced users are able to utilise the | ||
| tool quicker and to the same effect as the first approach. As a result, the 2nd approach was chosen as a better fit of the requirements | ||
| and in favour of having an application that is highly object-oriented. |
There was a problem hiding this comment.
Good to see that all aspects for the browsing have been well thought out and considered before making your decision on the implementation to go for. Well documented as well!
yAOwzers
left a comment
yAOwzers
left a comment
There was a problem hiding this comment.
Overall good job for the DG! There are some formatting issues like irregular spacings between titles and headings, but other than that 🥇
| ## 1. Introduction | ||
| **AniChan** is a command-line application written in **Java 11**. It is written using the Object-Oriented Programming (OOP) paradigm which provides us with means to structure a software program into organized, reusable and reusable pieces of code that makes it good for future improvements and revisions. |
There was a problem hiding this comment.
Would it be better if your team included a bit more details of what this application does/functions?
|
|
||
| <br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
Was the intention of the arrows from the main component to not be connected (touching) the other components? At the current moment, I feel a little unsure of where the main arrows should be pointing at.
|
|
||
| *Figure 1: Architecture Design Diagram* | ||
|
|
||
| > :bulb: The images used are stored in the directory: `images/`. If you wish to update a diagram you may replace the images in this folder. |
There was a problem hiding this comment.
It might be good to indicate what the light bulb image means at the beginning of the DG to help the user understand the logos and images used in this document!
| | Attribute | Option | Function | | ||
| | --- | --- | --- | | ||
| | order | 0 | Ascending | | ||
| | order | 1 | Descending | | ||
| | sortType | 0 | No Sort | | ||
| | sortType | 1 | by name | | ||
| | sortType | 2 | by rating | | ||
| | sortType | 3 | back to original | | ||
| | page | \>= 1 | page number | |
There was a problem hiding this comment.
I like this concept of using a table as information is presented in a very readable and simple manner. The only question I have is with regards to the heading. Maybe it would be better if you explained to the user what the headings were before introducing them? If not it might lead to further confusion.
| **Step 4:** Now `BrowseCommand` will utilise its `BrowseCommand#buildBrowseOutput()` operation to access all | ||
| `Anime objects within the page window, as shown in the diagram below. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Object diagrams do make it very simple for developers to understand the entire structure of a said component/function. From what I gathered, your team is explaining steps with respect to a given example. Would a sequence diagram be better at facilitating the flow of information since the users can read in a linear flow?
|
|
||
| Here is the sequence diagram to better illustrate the lifecycle of a browse command. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Nice sequence diagram to illustrate the entire lifecycle of the browse command! I'm not sure if it's my device but there seems to be some missing arrows for return values? Or is that the intended course of action for the various methods that were called? Also, the activation bar should end at appropriate life cycles, however there are a few activation bars that are left elongated.
|
|
||
| **Step 2:** The `Parser` class would then extract out `info` from the input given, which will instantiate a new `InfoParser` object, in which `InfoCommand` object is constructed as well. | ||
|
|
||
| **Step3:** The `InfoParser#parse()` method will be invoked, and this will validate the parameters given by the user. Once validated, ANIME_ID will be set inside the `InfoCommand` object that was created previously. `InfoCommand` object will be returned back all the way to `Main`. |
There was a problem hiding this comment.
There might be an overlooked irregular spacing here!
|
|
||
| > :memo: The other options (`-s`, `-l`, `-d`) follows a similar process, only the list and switch option does not interact with `StorageManager` and `Watchlist`. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Just to check, the box seems to be missing the label (eg. alt, opt)
There was a problem hiding this comment.
Hello, may I know which box you're referring to?
| | Approach | Pros | Cons | | ||
| | --- | --- | --- | | ||
| | After each command execution **(current design)**. | User don't have to worry about lost data if their application or system crashes midway. | Application might slow down when the data grows large. | | ||
| | When the user exits the program. | Saving is more efficient and could improve performance. | User may lose their data if the application or system crashes midway. | |
There was a problem hiding this comment.
Simple to read and understand! Good job 👍
| The following sequence diagram shows how the `Add Bookmark` operation works: | ||
|
|
||
|  <br/> | ||
| *Figure 27: Bookmark Add Command Sequence Diagram* |
There was a problem hiding this comment.
Good job on the sequence diagram, everything looks to be in-check! Easy to understand 👍
Hello, we have renamed the PR. Thank you! |
MuhammadHoze
left a comment
MuhammadHoze
left a comment
There was a problem hiding this comment.
Overall, good use of the tables and diagrams, maybe you could include them for most if not all of the implementations. Elaboration of each implementation is done clearly as well.
|
|
||
| <br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
I suggest you could put main in the middle and connect the rest of the relevant boxes to it rather than the floating arrows which may arise confusion.
| * `User`: Manages the workspace(s) and user data. | ||
| * `AnimeData`: Provides data from the anime source file. | ||
| * `Storage`: Reads data from, and writes data to, the hard disk. | ||
|
|
There was a problem hiding this comment.
Nicely explained here what each class does.
| **Step 4:** The `ViewWatchlistCommand#execute()` would then be called by `Main`, in which the WATCHLIST_ID will be validated, and if valid, a string containing all the anime name inside the active `Watchlist` will be built and returned to `Main`, where it will be printed out by `Ui`. | ||
|
|
||
| **Step 5:** `ViewWatchlistParser`, `ViewWatchlistCommand`, `Parser` and `Command` are terminated | ||
|
|
There was a problem hiding this comment.
I suggest maybe having a diagram for certain steps here like for your other implementations for better understanding
|
|
||
| The Bookmark class uses three ArrayList to store bookmark entries of the user, these arraylists maintain information about the anime index, episode and notes. The synchronisation between arraylist is required so that it enables easy retrieval of bookmark information using the bookmark index on the three arraylist. | ||
|
|
||
|  <br/> |
There was a problem hiding this comment.
Well done here! got a better picture on this implementation
| <br/> | ||
|
|
||
| ### 3.7 Storage Component | ||
|  <br/> |
There was a problem hiding this comment.
Clear view of all the class diagram
|
|
||
| <br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
Where are the arrows from the main intended to point to? It might be better if the arrows point more specifically.
| The UI component consists of a `UI` class that handles all user input and system output. The UI is only dependent on the `Main` class and does not interact directly with other classes ensuring high cohesiveness and separation of roles. | ||
|
|
||
| The `Ui` component listens for: | ||
| * the execution of commands to print the result of the Command. |
There was a problem hiding this comment.
Perhaps the Command could use the Command markdown instead since it is a major component, and also looks more consistent with the rest of the DG.
|  <br/> | ||
| *Figure 3: UI Class Diagram* | ||
|
|
||
| The UI component consists of a `UI` class that handles all user input and system output. The UI is only dependent on the `Main` class and does not interact directly with other classes ensuring high cohesiveness and separation of roles. |
There was a problem hiding this comment.
It might be better UI could use the UI markdown as well to maintain the consistency of the markdowns.
| | --- | --- | --- | | ||
| | 1. Leaving the list unsorted | - No complexity and fastest approach | - List will be unsorted and may cause confusion to users | | ||
| | 2. Resorting the list again | - The list will be back into its original form before browsing | - May hinder performance as resorting could take time <br/> - Requires altering of the main list | | ||
| | 3. Cloning a duplicate animeData object to sort | - The list will be back to its original form <br/> - The main list will not be affected at all | - Expensive operation that will require large storage and time complexity | |
There was a problem hiding this comment.
The use of a table is appropriate but it is a little wordy. Perhaps some icons could be used to increase the readability of the table.
…anualTesting-Docs
Update PPP and Developer Guide
…into EyoWeiChin-PPP-update # Conflicts: # docs/team/xinbin.md
…anualTesting-Docs
Merge EyoWeiChin's PPP Update
…g-Docs Merge ChanJianHao-V21-ManualTesting-Docs
…orkspaceLoad-Validation
…UpDiagram Merge Update the image to reflect new changes
…or-Fix Merge ChanJianHao-V21-Javadocs-Minor-Fix
Update UG and DG
Update DG phrasing
Merge Update UG to change release link to V2.1
Fix minor bug in UG
Minor bug fix on UG
Add Library Acknowledgement
13 participants
AniChan is an all-rounded tool to effectively create and organise anime lists with viewing statistics, efficiency-focused features, and tools to improve anime-watching experience.