[CS2113T-F12-1] CardLI #47

xRossKoh · 2021-10-01T04:24:07Z

CardLI is a desktop app for creating, organising, and reviewing flashcards via a Command Line Interface to serve students.

renzocanare

Overall, great work on the Developer Guide and use of sequence diagrams to supplement your wordier sections. With a little bit of quick-fixes on the formatting, the whole Developer Guide will look fantastic!

renzocanare · 2021-10-27T03:19:34Z

docs/DeveloperGuide.md

+### Storage Component
+The Storage component:
+* Saves all the decks
+* Saves all the flashcards
+* Remembers which deck each flashcard belongs to
+* Saves the results of each test


You can consider adding a sequence diagram here to show how the data is loaded into the app and saved into the text files. This will help your users to understand more about how the data is safely saved, and also explain your earlier point in the Design section on "initializes the components in the correct sequence".

renzocanare · 2021-10-27T03:31:51Z

docs/DeveloperGuide.md

+<!DOCTYPE html>
+<html>
+
+<head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+</head>
+
+<body>
+
+![](assets/Cards_CardLI.txt%20Example.png)
+</body>
+
+For the saving of the user's test history, the method call will expect an `ArrayList` of `AnswerList` objects along
+with a `type` argument of "tests".
+The `toString()` methods within the `AnswerList`and `Answer` classes have been overridden as per the specified format
+of saving the test history to the text file.
+For a `Answer` instance, the `toString()` method outputs a formatted string: `<answer> | <questionIndex>`. 
+For a `AnswerList` instance, the `toString()` method also outputs a formatted string containing information
+about the test deck and the user's test score, on top of information on each of the user's answers for the test. 
+An example of the format of the `Tests_CardLI.txt` where the decks of flashcards are saved is shown
+in the screenshot below.
+
+<!DOCTYPE html>
+<html>
+
+<head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+</head>
+
+<body>
+
+![](assets/Tests_CardLI.txt%20Example.png)
+</body>
+
+
+`readCardsFromFile()` and `readTestsFromFile()`


This section appears to have been formatted incorrectly and might have caused the rest of the DG to break. The image below is what this section looks like at your GitHub Pages DG link.

renzocanare · 2021-10-27T03:41:41Z

docs/DeveloperGuide.md

+![class diagram](../docs/assets/testClassDiagram.png)
+
+Currently, test feature is implemented on a systemwide level and is handled by `TestManager`.
+`TestManager` will call on `TestUi` and `TestParser` to handle the inputs and outputs with the user
+and the parsing respectively during the test.
+
+![sequence diagram](../docs/assets/TestSequenceDiagram.png)
+
+To enter into test mode, the user needs to enter `test` in main menu in which the program will
+prompt the user to input a number corresponding to the index of the deck that they want to 
+be tested on or "all" to test all decks.
+
+![sequence diagram](../docs/assets/getTestDeckSequenceDiagram.png)
+
+In both cases, `TestManager` will create an `AnswerList` using a `Deck` that it creates or gets from
+`DeckManager` depending on the condition which is shown by the sequence diagram above. 
+If the user decides to test all decks, the program will compile all `FlashCard` into a `Deck`. If the user
+decides to test a single deck, the program will get that deck instance from `DeckManager`.
+The `AnswerList` is where the user's response to the test is stored, and it is made up 
+of `Answer` as shown in the class diagram above. The `AnswerList` is also tagged with the test deck.
+
+![sequence diagram](../docs/assets/testAllCardsShuffledSequenceDiagram.png)
+
+After initializing the `AnswerList`, the testing begins. The `Deck` gets shuffled, then the cards (question)
+will be printed one at a time for the user to answer. The user's answer is then parsed and then added into
+the `AnswerList`. This process is repeated for the entire `Deck` that is being tested.
+
+![sequence diagram](../docs/assets/markTestSequenceDiagram.png)


Great choice to use sequence diagrams here, but it seems that the sequence diagrams are not appearing correctly in the GitHub Pages for your DG (as seen below).

You may want to consider removing ../docs/ from the image source in the code (e.g. assets/testClassDiagram.png as the image link can be relative to where DeveloperGuide.md is stored.

renzocanare · 2021-10-27T03:51:41Z

docs/DeveloperGuide.md

+
+Given below is the sequence diagram for `find`:
+
+![](assets/findFlashcardDiagram.png)


Good choice to add single-level numbering as it helps me to understand the sequence of which the calls flow. However, I personally am unsure if this is standard notation, and you may want to check if this is allowed. If it is, it's great help!

However, the image also appears quite small on your DG Page, and requires a few zoom-ins to read. If this is possible to adjust, it will definitely help the overall readability of your DG!

renzocanare · 2021-10-27T03:54:37Z

docs/DeveloperGuide.md

+
+`returnMatchingFlashCards()` is implemented by creating a stream that consists of all the `FlashCards` from one deck, and filters them based on whether they contain the search term given. Finally all the `FlashCards` that contain the search term are collected in an arrayList and their console outputs are returned in string format for `CardLiUi` to display to the user.
+
+Given below is the sequence diagram for `find`:


You may want to consider moving your sequence diagram for find to below the header as that is where many will expect to find images related to a whole section (and to keep in line with the formatting of the rest of your DG, where their sequence diagrams are located below the header of that section).

aditichadha1310 · 2021-10-27T04:08:20Z

docs/DeveloperGuide.md

+
+### `EditCardCommand`
+
+![](assets/editCardCommandSeqDiagram.png)


You might want to eliminate the lifeline of the activated bars once they are no longer being referenced.
I mean that once there is a cross in the line, which indicates the item is no longer being referenced you can choose to not extend the line after that cross.

aditichadha1310 · 2021-10-27T04:10:17Z

docs/DeveloperGuide.md

+
+### Move
+
+![](assets/moveCardCommandSeqDiagram.png)


Similarly here, your team might want to not extend the timeline after there is a cross (which represents that the object is no longer being referenced).

aditichadha1310 · 2021-10-27T04:18:11Z

docs/DeveloperGuide.md

+
+### Test Feature
+
+![class diagram](../docs/assets/testClassDiagram.png)


It seems that the image here is missing. You all might want to check this and update the DG.

aditichadha1310 · 2021-10-27T04:22:31Z

docs/DeveloperGuide.md

+
+How the parsing works:
+
+`Inner Parser` or `Outer Parser` creates a `XYZCommand` which in turn creates its corresponding `XYZCommandParser` (eg. when `InnerParser` creates a `EditCardCommand`, `EditCardCommand` creates a `EditCardParser`.)</li>


Your team might consider checking the content presented on the DG. For example, here the DG displays a List tag which is not supposed to be displayed.
Otherwise, its a good job done on the content:)

aditichadha1310 · 2021-10-27T04:26:42Z

docs/DeveloperGuide.md

+3. The command communicates with the `Model` when it is executed (eg. to add a flashcard).
+4. The result of the command execution is encapsulated as a `CommandResult` object which is returned to the `UI` from `Logic` to process.
+
+![](assets/parserArchitectureDiagram.png)


Your team might consider making the direction of these little triangle arrows more explicit for better understandability and redability.

aditichadha1310 · 2021-10-27T04:30:34Z

docs/DeveloperGuide.md

-|v2.0|user|find a to-do item by name|locate a to-do without having to go through the entire list|
+| |user| add flashcards|
+| |user| delete flashcards|
+| |user| view my flashcards|know what cards I currenly have in the deck|
+| |user| test myself with my flashcards|know if I have memorised the flashcards correctly|


As from the syntax used, I think in this section your team was trying to put forward the information in tabular form. However there are some formatting issues which do not present in the tabular form. I have faced a similar issue before and I think its because too much information is being presented in one line.
You can consider making the points (which are presented on each line) to be short so that it fits well in the table and can be presented in tabular form.

MAZJ124 · 2021-10-28T01:11:00Z

docs/DeveloperGuide.md

@@ -1,29 +1,366 @@
 # Developer Guide


Currently on Github pages, the template used for this developer guide only takes up half of the screen (at least on my device), perhaps it is good to change to a template that is easier for the reader to view?

MAZJ124 · 2021-10-28T01:15:04Z

docs/DeveloperGuide.md

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


Perhaps your team can add something like a content page here containing hyperlinks to jump to each section of the DG? This might make navigating around the document easier.

MAZJ124 · 2021-10-28T01:18:36Z

docs/DeveloperGuide.md

+
+### Model Component
+
+![](assets/modelArchitectureDiagram.png)


Maybe the dotted line with these arrows can be straight instead (especially when there is enough space) since the direction of the arrow might seem a little bit confusing to the reader.

MAZJ124 · 2021-10-28T01:27:59Z

docs/DeveloperGuide.md


-## Design & implementation
+![](assets/moveCardCommandSeqDiagram.png)


Having a sequence diagram here really helps to clear things up, however, unlike the previous diagrams, this one (and the next few) tends to have more content inside, and it is kind of hard for the reader to read, perhaps the size of the image can be enlarged?
On top of that, I think it was also mentioned by someone else as well: I believe that the lifeline of an object do not need to be drawn after the cross.

LouisLouis19 · 2021-10-28T02:06:44Z

docs/DeveloperGuide.md

@@ -1,29 +1,366 @@
 # Developer Guide

+# Introduction


Would it be better for users to navigate around if you consider creating a
"table of contents" section with hooks (a.k.a. hyperlinks)?

LouisLouis19 · 2021-10-28T02:10:39Z

docs/DeveloperGuide.md

+
+Given below is the sequence diagram for `edit` (Deck):
+
+![](assets/editDeckCommandSeqDiagram.png)


You could perhaps consider using create in your sequence diagrams to indicate newly created objects.
You could consider this for DeckManager, Deck, or CardLiUi

LouisLouis19 · 2021-10-28T02:11:10Z

docs/DeveloperGuide.md

+### `EditDeckCommand`
+
+
+Given below is the sequence diagram for `edit` (Deck):


You could perhaps consider deleting the lifeline upon object deletion

LouisLouis19 · 2021-10-28T02:19:10Z

docs/DeveloperGuide.md

+
+Given below is the sequence diagram for `find`:
+
+![](assets/findFlashcardDiagram.png)


More of a consistency issue, perhaps you could adjust the sequence diagrams for move, find, and editcardcommand to follow the same style. For instance, in object deletion, if you want to use the cross, consider using it for all your diagrams.

For the find sequence diagram in particular, well done in using the numbering, perhaps this could be replicated in the other diagrams as well. Also, "create" is used here, which again could be replicated in the other sequence diagrams if applicable.

You could also take an extra step by adding numbered steps in the explanation and adjusting it according to the numbers in the sequence diagram

LouisLouis19 · 2021-10-28T02:20:48Z

docs/DeveloperGuide.md

+returns `deck`, of string type, which represents the deck to be edited.`prepareMoveCommand()` will then return
+a string array, `preparedArguments`, which represents the arguments for the next method call.
+
+The `execute()` method will then call the `moveCard()` method of the `DeckManager` class, which in turn calls the


The sequence diagram explanation is detailed, to enhance it further, you could consider using bullet points to ease readers in understanding the thought process better.

LouisLouis19 · 2021-10-28T02:23:10Z

docs/DeveloperGuide.md

+prompt the user to input a number corresponding to the index of the deck that they want to 
+be tested on or "all" to test all decks.
+
+![sequence diagram](../docs/assets/getTestDeckSequenceDiagram.png)


Unfortunately, the class and sequence diagrams for the Test feature are not rendered properly. Could try to fix this urgently

LouisLouis19 · 2021-10-28T02:24:29Z

docs/DeveloperGuide.md

+namely an `ArrayList` of a generic type `<T>` as well as a `boolean` value, `saveCards`, that indicates whether the method
+is saving cards or tests to their respective text files.
+
+![](assets/writeToFileSequenceDiagram.png)


Similar to the previous sequence diagrams, perhaps you could consider making your diagram styling more consistent.

LouisLouis19 · 2021-10-28T02:27:52Z

docs/DeveloperGuide.md

+namely an `ArrayList` of a generic type `<T>` as well as a `boolean` value, `saveCards`, that indicates whether the method
+is saving cards or tests to their respective text files.
+
+![](assets/writeToFileSequenceDiagram.png)


You could perhaps consider using "create" for newly created objects, ie. FileWriter, ArrayList

LouisLouis19 · 2021-10-28T02:31:03Z

docs/DeveloperGuide.md

+* Types fast
+* Prefers to store their information online rather than physically
+* Has a lot of flashcards
+

 ### Value proposition


Well done in keeping this section concise. However, a bug caused the page to not render properly.
The user stories, non-functional requirements, glossary, and instructions for manual testing sections are not rendered as expected.

LouisLouis19 · 2021-10-28T02:36:52Z

docs/DeveloperGuide.md

+
+Given below is the sequence diagram for `find`:
+
+![](assets/findFlashcardDiagram.png)


You could perhaps consider adding the operation invoked above the arrows in steps 2 and 3

EdisonZhong17 · 2021-10-29T03:27:53Z

docs/DeveloperGuide.md

+CardLI is a Command Line Interface (CLI) desktop app designed to help students manage their flashcards. CardLI can help
+students keep track of all their flashcards. It also allows students to test their knowledge. All of this in one
+single platform.
+


Before the start of DG, it would be better to have a set of hyperlinks for user to navigate the whole DG.

EdisonZhong17 · 2021-10-29T03:45:12Z

docs/DeveloperGuide.md

+returns `deck`, of string type, which represents the deck to be edited.`prepareMoveCommand()` will then return
+a string array, `preparedArguments`, which represents the arguments for the next method call.
+
+The `execute()` method will then call the `moveCard()` method of the `DeckManager` class, which in turn calls the


In this part can consider to use FUNCTION_NAME to make the long paragram more clearer.

EdisonZhong17 · 2021-10-29T03:47:48Z

docs/DeveloperGuide.md

+
+### Test Feature
+
+![class diagram](../docs/assets/testClassDiagram.png)


some of the diagrams are not showing properly, maybe there's something wrong when you imported this .png file to your DG.

EdisonZhong17 · 2021-10-29T03:51:37Z

docs/DeveloperGuide.md

+the method. 
+
+![](assets/readTestsFromFileSequenceDiagram.png)
+


For this sequence diagram, maybe you can consider adding cross after each activation bar is ended.

…mer not working properly in intellij

Edited find function to return error message when there are no cards matching the search term.

changed split parameters such that there needs to be a space after a flag

Fixed dg bugs and bugs from PED

Fix output when viewing a flashcard to accommodate long lines

Changed storage and test functionalities

Bug fixes for v2.1

Fixed bugs for EditDeck,EditCard,UG; Added Junit Testing for EditDeck,EditCard

final updates to dg

made diagrams smaller

Edit Countdown sequence diagram

fixed dg

Fixed dg

Fix bug when adding command

added javadocs

Add Javadoc for CommandArgumentsParser class

w

sorted out authorship in ugdg. updated ppp

renzocanare reviewed Oct 27, 2021

View reviewed changes

aditichadha1310 reviewed Oct 27, 2021

View reviewed changes

MAZJ124 reviewed Oct 28, 2021

View reviewed changes

LouisLouis19 reviewed Oct 28, 2021

View reviewed changes

EdisonZhong17 reviewed Oct 29, 2021

View reviewed changes

xkisxk and others added 25 commits November 1, 2021 17:03

Fixed bug where commands with no input accepts input

df6abc1

Fixed bug

056ce4b

implemented 2dp printing for test result but could not test due to ti…

f1ef25c

…mer not working properly in intellij

solved checkstyle issues

26a22b3

Edit find function to return error message

3a9d1f6

Edited find function to return error message when there are no cards matching the search term.

Handle large indexes, delete the delete by input function

02a3451

Added more testing

98a4904

Fixed more bugs in delete card and edit card

cb34f3d

Fixed UG bug and EditDeck bug

750ca31

Handled very big integer strings with number format exception

0dacfbd

fixed checkStyletest

bc5ec00

fiexed EditCard bugs

af60c22

fixed out of bounds error for EditCardCommand

f19b604

change split parameters

d3fce08

changed split parameters such that there needs to be a space after a flag

Merge pull request #166 from xkisxk/fix-bugs

ffae66d

Fixed dg bugs and bugs from PED

Merge branch 'master' into master

c7abf17

Merge pull request #168 from astralum/master

6a8c7e9

Fix output when viewing a flashcard to accommodate long lines

Merge branch 'master' into json

0411a51

Merge pull request #171 from xRossKoh/json

7062e59

Changed storage and test functionalities

Merge branch 'master' into fix-bugs

1adc377

Merge pull request #172 from JWweiyin/fix-bugs

b4362da

Bug fixes for v2.1

Merge branch 'master' into week-11

7e58337

Merge pull request #173 from ThaddeusLim99/week-11

fa2c13a

Fixed bugs for EditDeck,EditCard,UG; Added Junit Testing for EditDeck,EditCard

fix commands that dont take other arguments

335db05

Merge branch 'week-11'

cbcfdb9

xRossKoh and others added 30 commits November 8, 2021 19:42

Update xrosskoh.md

65a8bfd

Merge pull request #213 from xkisxk/update-dg

b35f4fc

final updates to dg

Update xrosskoh.md

f82bb53

Update jwweiyin.md

5235bba

Update thaddeuslim99.md

f453011

made uml diagrams smaller

f33bed9

Merge branch 'master' into master

51c1f90

Merge pull request #214 from JWweiyin/master

99b9c4f

made diagrams smaller

Change colour of class in UML sequence diagram

d02e23f

Merge branch 'master' of https://github.com/AY2122S1-CS2113T-F12-1/tp

7652d7a

Merge branch 'AY2122S1-CS2113T-F12-1:master' into master

aa3edfe

Merge pull request #215 from astralum/master

435cd52

Edit Countdown sequence diagram

fixed dg

b24746a

Merge pull request #216 from xkisxk/update-dg

68e9d23

fixed dg

Update UserGuide.md

8460e6f

Merge branch 'master' of https://github.com/AY2122S1-CS2113T-F12-1/tp

9b011ce

Fixed dg

54d8b51

Merge pull request #217 from xkisxk/update-dg

ca15a90

Fixed dg

Fix bug when adding command

70df475

Merge branch 'master' of https://github.com/AY2122S1-CS2113T-F12-1/tp

b79670e

Merge pull request #218 from astralum/master

2b87579

Fix bug when adding command

Add Javadoc for CommandArgumentsParser class

ceb1397

added javadocs

efce153

Merge pull request #220 from xkisxk/master

08aeae5

added javadocs

Merge pull request #219 from astralum/master

a5b2aaf

Add Javadoc for CommandArgumentsParser class

fix check erros

a72180d

Merge branch 'master' of https://github.com/xkisxk/tp

7c6854d

Merge pull request #221 from xkisxk/master

45572bf

w

sorted out authorship in ugdg. updated ppp

bda5c4b

Merge pull request #222 from xkisxk/master

974ee7f

sorted out authorship in ugdg. updated ppp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CS2113T-F12-1] CardLI #47

[CS2113T-F12-1] CardLI #47

xRossKoh commented Oct 1, 2021

renzocanare left a comment

renzocanare Oct 27, 2021

renzocanare Oct 27, 2021

renzocanare Oct 27, 2021

renzocanare Oct 27, 2021

renzocanare Oct 27, 2021

aditichadha1310 Oct 27, 2021

aditichadha1310 Oct 27, 2021

aditichadha1310 Oct 27, 2021

aditichadha1310 Oct 27, 2021

aditichadha1310 Oct 27, 2021

aditichadha1310 Oct 27, 2021

MAZJ124 Oct 28, 2021

MAZJ124 Oct 28, 2021

MAZJ124 Oct 28, 2021

MAZJ124 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

LouisLouis19 Oct 28, 2021

EdisonZhong17 Oct 29, 2021

EdisonZhong17 Oct 29, 2021

EdisonZhong17 Oct 29, 2021

EdisonZhong17 Oct 29, 2021


		Given below is the sequence diagram for `find`:

		![](assets/findFlashcardDiagram.png)


		`returnMatchingFlashCards()` is implemented by creating a stream that consists of all the `FlashCards` from one deck, and filters them based on whether they contain the search term given. Finally all the `FlashCards` that contain the search term are collected in an arrayList and their console outputs are returned in string format for `CardLiUi` to display to the user.

		Given below is the sequence diagram for `find`:


		### `EditCardCommand`

		![](assets/editCardCommandSeqDiagram.png)


		### Test Feature

		![class diagram](../docs/assets/testClassDiagram.png)


		How the parsing works:

		`Inner Parser` or `Outer Parser` creates a `XYZCommand` which in turn creates its corresponding `XYZCommandParser` (eg. when `InnerParser` creates a `EditCardCommand`, `EditCardCommand` creates a `EditCardParser`.)</li>


		## Design & implementation
		![](assets/moveCardCommandSeqDiagram.png)


		Given below is the sequence diagram for `edit` (Deck):

		![](assets/editDeckCommandSeqDiagram.png)

		### `EditDeckCommand`


		Given below is the sequence diagram for `edit` (Deck):

[CS2113T-F12-1] CardLI #47

Are you sure you want to change the base?

[CS2113T-F12-1] CardLI #47

Conversation

xRossKoh commented Oct 1, 2021

renzocanare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment