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

[CS2103-F10-3] CS2103/T Revision Tool #97

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

Conversation

khiangleon
Copy link

@khiangleon khiangleon commented Sep 27, 2019

|`*` |lonely CS2103 student |have someone to talk to, even if it’s a computer |I won't feel lonely

|`*` |CS2103 student who keeps having stomach ache |the application to tell me where the nearest toilet is |go and shit
|=======================================================================

[appendix]
== Use Cases

Choose a reason for hiding this comment

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

Did you forget to update the Use Cases?

Copy link

@ShirleyWangxt ShirleyWangxt left a comment

Choose a reason for hiding this comment

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

In general, the diagrams have used intuitive symbols. The description given is understandable and sufficiently high-level. Diagrams are not cluttered with too many unnecessary details. The document uses multiple types of diagrams and they are well-integrated into the description. There are some details to be fixed, but the document looks neat, well-formatted, and professional overall.

@@ -295,20 +267,64 @@ Priorities: High (must have) - `* * \*`, Medium (nice to have) - `* \*`, Low (un
[width="59%",cols="22%,<23%,<25%,<30%",options="header",]
|=======================================================================
|Priority |As a ... |I want to ... |So that I can...
|`* * *` |new user |see usage instructions |refer to instructions when I forget how to use the App

Choose a reason for hiding this comment

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

The user stories are correctly formatted. All three parts are present.


|`* * *` |CS2103 student with a lot of things on my mind |mark certain questions that I am unsure of |refer back to the question when I am free.

|`* * *` |CS2103 student |import questions from my peers |study on my own.

Choose a reason for hiding this comment

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

The benefit does not match the function. It would be better if the benefit is more specific, such as the student does not need to input the questions himself/herself.


|`* * *` |conscientious CS2103 student |export the questions I am unsure of |raise them up during tutorials.

|`* * *` |indecisive student |be recommended questions instead of me having to plan my own study plan |go directly to studying

Choose a reason for hiding this comment

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

The benefit is a bit unclear. Hard to see how it matches the function.


|`* *` |competitive CS2103 student |at least know where I stand among my cohort |look at who is the next person I can beat.

|`* *` |gamer CS2103/T student |accomplish tasks that give me a sense of achievement, preferably through in application rewards |I feel good.

Choose a reason for hiding this comment

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

The benefit can be more specific like I can more motivated to accomplish more tasks.

* can type fast
* prefers typing over mouse input
* is reasonably comfortable using CLI apps

*Value proposition*: manage contacts faster than a typical mouse/GUI driven app
*Value proposition*: helps student to ace CS2103/T

[appendix]
== User Stories

Choose a reason for hiding this comment

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

Some of the user stories might be a little irrelevant to this app.


By: `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT`
By: `Group F10-3`      Since: `Aug 2019`      Licence: `MIT`

== Setting up

Choose a reason for hiding this comment

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

For Figure 2 Class Diagram of the Logic Component, it would be better if the classes and arrows and the different sections can be color-coded to make it easier for visualisation.

@@ -98,9 +98,9 @@ image::LogicClassDiagram.png[]
*API* :

Choose a reason for hiding this comment

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

For Figure 3. Component interactions for delete 1 command, the arrow seems to overlap with the activation bar.
Screenshot 2019-10-25 at 11 37 09 AM

For the self-invocation part, it might be better if the line in the middle can be covered.
Screenshot 2019-10-25 at 11 40 03 AM

Copy link

@irene-lly irene-lly left a comment

Choose a reason for hiding this comment

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

Overall good job, the diagrams are neat and easy to understand, the document is also pretty well-formatted.

* stores the Address Book data.
* exposes an unmodifiable `ObservableList<Person>` that can be 'observed' e.g. the UI can be bound to this list so that the UI automatically updates when the data in the list change.
* stores a `Question Bank` object that represents the Question Bank.
* stores the Test Bank data.

Choose a reason for hiding this comment

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

A markup can be used here i.e. Test Bank

@@ -121,13 +121,13 @@ image::ModelClassDiagram.png[]

Choose a reason for hiding this comment

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

If QuestionList implements ReadOnlyQuestionList then ReadOnlyQuestionList should be an abstract class or an interface.
1

Comment on lines 196 to 198
* **Alternative 1 (current choice):** Read and parse the command to execute it
** Pros: Easy to implement.
** Cons: May have performance issues in terms of memory usage.
Copy link

Choose a reason for hiding this comment

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

Maybe it is better to come up with an alternative 2 implementation for comparison?


Step 1. The user launches the application for the first time. The `VersionedAddressBook` will be initialized with the initial address book state, and the `currentStatePointer` pointing to that single address book state.
Step 1. The user types `add mcq q/"string of question" x/option1 x/option2 y/option3 x/option4 cat/[UML] diff/[easy]`, this command adds a easy difficulty mcq question about UML with 4 options and option3 being the correct answer.

image::UndoRedoState0.png[]

Choose a reason for hiding this comment

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

The diagrams seems not up to date. Does it match your description?

If the `currentStatePointer` is at index 0, pointing to the initial address book state, then there are no previous address book states to restore. The `undo` command uses `Model#canUndoAddressBook()` to check if this is the case. If so, it will return an error to the user rather than attempting to perform the undo.

The following sequence diagram shows how the undo operation works:
The following sequence diagram shows how the add question operation works:

image::UndoSequenceDiagram.png[]

NOTE: The lifeline for `UndoCommand` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram.

Choose a reason for hiding this comment

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

Not updated.


* **Alternative 1 (current choice):** Saves the entire address book.
* **Alternative 1 (current choice):** Read and parse the command to execute it

Choose a reason for hiding this comment

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

Any alternatives to your current choice?

Choose a reason for hiding this comment

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

Is it possible to be more specific when describing the pros and cons?

alages97 pushed a commit to alages97/main that referenced this pull request Oct 27, 2019
Documentation for Statistics and Notes
wilfredbtan and others added 19 commits October 31, 2019 22:43
Merged Shaun, Sihao. Partial merge of Junxian's Timer and CustomMode. Updated UI as well.
…into AutoComplete-feature

# Conflicts:
#	docs/UserGuide.adoc
#	src/main/java/seedu/revision/logic/commands/main/StartQuizCommand.java
…into branch-status

* 'master' of https://github.com/AY1920S1-CS2103-F10-3/main: (30 commits)
  pass checkstyle
  enable ui to change between mcq, saq, truefalse
  minor ui updates
  added SaqAnswersGridPane to initialise an empty pane for Saq
  pass checkstyle
  clean up SampleDataUtil
  set Answer word limit and update SampleDataUtil
  resolved some travis issues
  add Custom Mode
  Edit StartQuizCommandParser to parse predicates
  update UserGuide, add customisable timer, extend Mode for NormalMode
  -
  Refactor CustomMode to factory to accomodate optional values for timer, category and difficulty
  Add CustomMode.java
  added javadoc for answerchecker class
  update saq AnswerChecker to check for numbers in number format and word format
  Shift order of code in startQuizWindow#executeCommand 1. update bar 2. check if next question is next stage, if yes, confirmation message then create new timer and bar if no, reset timer 3. update the question options and question text
  update QuizCommandParser to parse Saq input
  Fix progress bar to show 8/8 Fix popup window displaying when user inputs command Todo: popup when timer reaches 0 for last question of level
  Make timer stop after every command in StartQuizWindow#executeCommand
  ...

# Conflicts:
#	src/main/java/seedu/revision/ui/StartQuizWindow.java
…into branch-status

* 'master' of https://github.com/AY1920S1-CS2103-F10-3/main:
  Resolve bug in ArcadeMode
  Implement custom mode v1.0

# Conflicts:
#	src/main/java/seedu/revision/logic/commands/main/CommandResult.java
neoshh and others added 30 commits November 11, 2019 21:11
* master:
  update use case, copy UG DG into PPP
  add newline at EOF
  Update PPP
  Update autocomplete to remove double enter
  Resolve Wifred's comments on PR#162
  Updated DG appendix
  fix find bug where '?' character is considered a word
  update sequence diagrams for delete and quiz
  Update DG for restore and autocomplete
  update UG, DG, add design considerations and implementation details of Configuration and Quiz Mode
  improve code quality but renaming variables reorganising methods
  Implement SaqInputCommandParserTest Resolve issue #114
  Implement SaqTest
  Implement 6 passing tests for TrueFalseTest
  -
  Implement McqInputCommandTest Implement failing SaqInputCommandTest
  Implement TrueFalseInputCommandTest Resolve bug where t/f input commands not being marked as case insensitive

# Conflicts: RESOLVED
#	docs/DeveloperGuide.adoc
…into branch-charts

* 'master' of https://github.com/AY1920S1-CS2103-F10-3/main: (28 commits)
  resolved travis build issues
  Final PPP, DG, UG
  updated PPP
  updated sihao's PPP
  updated user guide
  updated dev guide
  update use case, copy UG DG into PPP
  Updated dev guide
  add newline at EOF
  Update PPP
  added Saq AnswerChecker activity diagram to DG
  Update autocomplete to remove double enter
  Added Saq isCorrect sequence diagram to DG
  Resolve Wifred's comments on PR#162
  Updated DG appendix
  fix find bug where '?' character is considered a word
  Updated developer guide with SaqAnswerChecker class diagram
  update sequence diagrams for delete and quiz
  Update DG for restore and autocomplete
  Add Ui overview in DG
  ...

# Conflicts:
#	docs/AboutUs.adoc
#	docs/DeveloperGuide.adoc
#	src/main/java/seedu/revision/ui/StartQuizWindow.java
updated about us to link profiles to correct PPP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.