[CS2113-F10-2] BadMaths by YC-Michael · Pull Request #46 · nus-cs2113-AY2223S2/tp

YC-Michael · 2023-03-03T02:29:09Z

BadMaths is a quick lookup tool for trigonometry graphs and basic matrix operations that aims to help students save time when doing maths assignments.

Fix edit index

wuzhzn

Perhaps more diagrams (class/architectural/sequence can be added to show the overall flow of the code. There can also be more details given in the scope, user stories, glossary and such if it is not included

wuzhzn · 2023-03-29T04:15:29Z

+1. This is step 1.
+2. This is step 2.
+
+![img.png](img.png)


Is this format of the sequence diagram correct? Should there be colon in the class such as :Notes?

wuzhzn · 2023-03-29T04:18:44Z

+## Product Scope
 ### Target user profile

 {Describe the target user profile}


Perhaps include target user profile

haoyangw

Very clear and detailed explanation given for your features! Would be good to read about why you implemented your logic this way, but great work nonetheless!! :)

haoyangw · 2023-03-30T04:12:37Z

+Quadratic -> Quadratic: findA()
+activate Quadratic #FFBBBB
+deactivate Quadratic
+Quadratic -> Quadratic: findB()
+activate Quadratic #FFBBBB
+deactivate Quadratic
+Quadratic -> Quadratic: findC()
+activate Quadratic #FFBBBB
+deactivate Quadratic
+Quadratic -> Quadratic: quadraticFormula()
+activate Quadratic #FFBBBB


Perhaps you can adjust the method call arrows for findA(), findB(), findC() and quadraticFormula() such that they point to the start of the corresponding activation bars, like what they arrow for printAnswer() does

haoyangw · 2023-03-30T04:16:10Z

+deactivate UI
+end opt
+deactivate Quadratic
+Quadratic -> Command


I believe in this case, the arrow should be a dashed arrow since it represents a method return. Also, you should try to connect the method return arrow to exactly the end of the activation bar for Quadratic

haoyangw · 2023-03-30T04:23:15Z

+deactivate Parser
+alt inputCommand is null
+activate Command
+    BadMaths -> Command : Command(command, toDo);


Perhaps this method invocation arrow should be connected to the constructor activation bar instead rather than a regular method activation bar. Maybe try adding create Command to display the constructor activation bar earlier in the PUML file?

haoyangw · 2023-03-30T04:26:23Z

+Command -> Quadratic : Quadratic(toDo)
+activate Quadratic
+deactivate Quadratic
+alt "Bye"


Maybe you can phrase it more like a condition, e.g. command is "Bye", so that it's clearer what this "Bye" is referring to

haoyangw · 2023-03-30T04:26:37Z

+activate Quadratic
+deactivate Quadratic
+alt "Bye"
+else "Graph"


Similar comment as for the "Bye" condition above

haoyangw · 2023-03-30T04:30:10Z

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

+
 ## Design & implementation


Concise and clear explanation of what your features does in this section! Consider explaining why you implemented your feature logic this way, maybe even describing some alternative implementations you considered but ultimately dropped, which would strengthen the appeal of your features to a developer.

haoyangw · 2023-03-30T04:31:32Z

+activate TrigoGraphAnalyser
+deactivate TrigoGraphAnalyser
+
+TrigoGraph -> TrigoGraph


I think this arrow should be a dashed arrow instead since it's a method return

haoyangw · 2023-03-30T04:32:15Z

+TrigoGraph -> TrigoGraph
+deactivate TrigoGraph
+
+TrigoGraph -->> TrigoGraphVisualiser: TrigoGraphVisualiser(amplitude,phase,frequency,verticalShift,trig)


I think this should be a solid arrow since it's a method call

haoyangw · 2023-03-30T04:32:39Z

+activate Ui
+deactivate Ui
+
+TrigoGraph -> TrigoGraph


I think this should be a dashed arrow since it's a method return

haoyangw · 2023-03-30T04:34:41Z

+
+#### TrigoGraph class:
+![img_1.png](img_1.png)
+Step 2. Constructor for the TrigoGraph class takes in `2*sin(2*x+5)-1` and assigns it to `trigoEqn` of type String. When `startGraphAnalysis()`


I think you need to insert another newline before Step 2. because the Step 2. line currently begins beside the UML diagram

SSzeWen

Overall, table of contents and format look great, maybe more could be put into the the return arrow back for sequence diagrams.

SSzeWen · 2023-03-30T04:19:15Z

+Step 5. If any exceptions are caught in the above steps, `printQuadraticFormulaError` would be called from UI to show an error message
+to the user.
+
+![img_3.png](img_3.png)


DG for the line quadraticFormula(), I think it would be better to include the variables to become something like
quadraticFormula(a,b,c)

SSzeWen · 2023-03-30T04:20:39Z

+Step 5. If any exceptions are caught in the above steps, `printQuadraticFormulaError` would be called from UI to show an error message
+to the user.
+
+![img_3.png](img_3.png)


Would it be better to also add return statements, such as the variables names the functions are returning for such as the findA(), findB(), and findC() fuctions

SSzeWen · 2023-03-30T04:31:19Z

+which will be stored in a list, and to display a list of all notes
+stored by users. This functionality is achieved through the `Store.` and `List.` commands.
+
+![img_2.png](img_2.png)


I think getCommand and getToDo for the DG at the top, the arrow should be the other way round cause it is badmaths that is calling parser's function, and the calls should have a separate activation bar

SSzeWen · 2023-03-30T04:38:51Z

+which will be stored in a list, and to display a list of all notes
+stored by users. This functionality is achieved through the `Store.` and `List.` commands.
+
+![img_2.png](img_2.png)


for trigoGraph(), Calculator() and Quadratic(), they are constructors which make a new object, would it be better to show it in the Sequence Diagram by linking it straight to the bar instead of those objects already existing?

Implement JUnit Test for CommandHistory.java

update imports for NotesFileWriterTest

Added PPP

Reflect tp review for the matrix part

Implement DeleteTest.java

Reflect tp review for the matrix part

Update draft PPP

Added Store Sequence Diagram

add header comment

Seq diagram rectify

Add Sequence Diagram for Store

Rectify UI output command for Invalid Number Entered

Update UserGuide.md

Refine Seungjun's PPP

Fix DG

Fix format

handle indentation problem

Update DG

Add JavaDoc comments for the matrix part

change main README.md to BadMaths version

kyrixn added a commit to kyrixn/tp that referenced this pull request Mar 16, 2023

Merge pull request nus-cs2113-AY2223S2#46 from Zemdalk/fix-edit-index

cb9d25b

Fix edit index

wuzhzn reviewed Mar 29, 2023

View reviewed changes

haoyangw reviewed Mar 30, 2023

View reviewed changes

SSzeWen reviewed Mar 30, 2023

View reviewed changes

CVRP-SOLE and others added 26 commits April 6, 2023 17:08

refactoring magic literals to improve readability

89d5cf1

Implement JUnit Tests for CommandHistory.java

8f6864d

Merge pull request #229 from ZiqiuZeng/branch-NotesFileWriter-JUnit-Test

5f7e0e6

update imports for NotesFileWriterTest

355c4b1

Merge pull request #237 from WilsonLee2000/TestJUnit

fee543b

Implement JUnit Test for CommandHistory.java

Improve readability of UG

1b80b65

Merge pull request #238 from ZiqiuZeng/branch-NotesFileWriterTest-Update

8f9eb9d

update imports for NotesFileWriterTest

no message

54b3dfd

Merge remote-tracking branch 'origin/test'

1704811

Merge pull request #234 from khooyourun/PPP

e8d1ef8

Added PPP

Merge pull request #240 from 0nandon/master

377beb4

Reflect tp review for the matrix part

no message

3a8d181

Implement DeleteTest.java

c4e26b7

removing comments that are private notes to me

5976f10

Implement JUnit Tests for List.java

2b72547

Merge pull request #243 from WilsonLee2000/TestJUnit

6fffb2c

Implement DeleteTest.java

refactoring long methods

fc3c8ec

Merge pull request #244 from 0nandon/master

683e833

Reflect tp review for the matrix part

Merge branch 'TestJUnit'

d461feb

Update draft PPP

eb53ad7

Merge pull request #248 from ZiqiuZeng/branch-Draft-PPP

43d46f9

Update draft PPP

delete unused imports

46f1669

refactor classes into corresponding packages

d02cf4f

handle redundant imports

c7b1455

Create seungjun_lee.md

c0a6d3a

Update seungjun_lee.md

557bd87

WilsonLee2000 and others added 30 commits April 10, 2023 20:05

Merge pull request #305 from WilsonLee2000/StoreSequenceDiagram

11ddfdb

Added Store Sequence Diagram

Merge pull request #304 from ZiqiuZeng/update-header-comment

6d7bf59

add header comment

Merge branch 'StoreSequenceDiagram'

5ed0428

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F10-2/tp

028412e

Remove Store Seq Diagram

e7be577

Merge pull request #306 from WilsonLee2000/seqdiagramrectify

446871d

Seq diagram rectify

Add Seq Diagram for Store

d726cb4

Merge pull request #307 from WilsonLee2000/SeqDiagramAdd

eeac598

Add Sequence Diagram for Store

Merge branch 'SeqDiagramAdd'

f15b2ec

Fix UI Naming Error

c99999a

Merge pull request #308 from WilsonLee2000/fixuibug

c346fc1

Rectify UI output command for Invalid Number Entered

Update UserGuide.md

477be31

Merge pull request #309 from 0nandon/master

43fc0f9

Update UserGuide.md

Update 0nandon.md

a0dddb0

Update 0nandon.md

187283a

Merge pull request #310 from 0nandon/master

8c1692e

Refine Seungjun's PPP

Update DeveloperGuide.md

a35469d

Update DeveloperGuide.md

1472079

Fix format

5041060

Merge pull request #311 from 0nandon/master

9a629fd

Fix DG

handle indentation problem

989821c

Merge pull request #312 from YC-Michael/master

195b515

Fix format

Merge pull request #313 from ZiqiuZeng/refine-dg

42496d5

handle indentation problem

Update DeveloperGuide.md

c726548

Update DeveloperGuide.md

8105036

Merge pull request #314 from 0nandon/master

4fb9cb4

Update DG

Add JavaDoc comments for the matrix part

7823156

Merge pull request #315 from 0nandon/master

7586d3f

Add JavaDoc comments for the matrix part

change main README.md to BadMaths version

c4bc080

Merge pull request #316 from 0nandon/master

db52353

change main README.md to BadMaths version

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


		## Design & implementation

Conversation

YC-Michael commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wuzhzn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haoyangw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SSzeWen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

YC-Michael commented Mar 3, 2023 •

edited

Loading