[CS2113-T13-3] WhereGotTime#36
Conversation
AmeliaTYR
left a comment
AmeliaTYR
left a comment
There was a problem hiding this comment.
I like that a lot of effort was put into making several diagrams explaining various components of the program and the aesthetic of some of the digitally hand drawn diagrams, though further elaboration will help enhance the usefulness of your diagrams and help the user better understand your code and implementation rationale. Good work!
| while the other two will result in the app giving an error message | ||
| prompting the user to re-enter again.<br/> | ||
|
|
||
| ### Edit Function<br/> |
There was a problem hiding this comment.
This segment seems incomplete? Minimally some description of what these functions are instead of simply listing would be helpful to the reader.
| #### Design of the Add Function<br/> | ||
|
|
||
| UML Class Diagram of the Add function: | ||
| <br/> |
There was a problem hiding this comment.
The UML diagram looked a bit strange with the large blank spaces within the classes and the much smaller font used for the multiplicities.
| #### Design of the Add Function<br/> | ||
|
|
||
| UML Class Diagram of the Add function: | ||
| <br/> |
There was a problem hiding this comment.
The filled in black arrow heads were to be avoided iirc? (inheritance or association issue?)
| and location) to the timetable class into its arraylist divided by the day of the week<br/> | ||
|
|
||
| #### Sequence Design of Add Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
You could perhaps indicate more clearly where the arrows are coming from or going.
For the sake of neatness you could try to do the diagrams in software like powerpoint some or uml creator or something instead of writing cause the pencil marks here are a bit messy sorry
There was a problem hiding this comment.
You could also perhaps use the same software to create your uml diagrams for the sake of consistency, as the diagrams vary in style along the document.
There was a problem hiding this comment.
- parsedInputs[] cannot be a method call.
- "Return" lines should orginate from the point of deactivation
- "new event" method call line should not point backwards in time. Perhaps you can reformat this diagram such that it is a straight line.
|
|
||
| #### System Architecture of the Add Function<br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
For this diagram it looks like the user can by pass the interface and directly add commands? Its a bit confusing whether this was intended or why it was designed this way. Further explanation on this part of the diagram would be helpful
|
|
||
| ### Compare Function<br/> | ||
| #### System Architecture of the Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
this diagram faces a similar issue of why the user can by pass the ui as mentioned above
There was a problem hiding this comment.
Same issue with the second "blue" user.
|
|
||
| ### Compare Function<br/> | ||
| #### System Architecture of the Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
For this diagram why are there two different coloured users and why do they have different access to the program? The purpose of drawing this additional user and their role is unclear, even in the explanation below.
The duplicated timetable box is also confusing.
|
|
||
|
|
||
| #### Sequence Design of Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
For this diagram it is a bit disorganized and confusing.
Firstly, it is a bit unclear whether the User class here refers to the User class in the compare function (in which case there shouldn't be a random method call coming from it) or if it refers to an actual user (in which case the notation should be the stick figure not the class header)
Secondly, from the CompareCommand activation bar has an unknown return coming from the middle of the activation bar, while the parsedInputs method call being made to the same activation bar is too low down. This is also the case with the activation bar for the self invoked method call execute and the common time array method call to the UI class.
It is also unclear where the method call to the UI intends to return to as it is chronologically further down than the end of the return from the parsedInputs method.
arindamshivatrip
left a comment
arindamshivatrip
left a comment
There was a problem hiding this comment.
ON the whole the diagrams are a little confusing. but well made and visually appealing.
| |v2.0|user|be assured that my inputs are correctly added|my input are correctly convey into the timetable| | ||
| |v2.0|user|access my saved timetables|I do not have to manually enter the timetables again| | ||
|
|
||
| ## Non-Functional Requirements |
|
|
||
|
|
||
| #### Sequence Design of Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
Here there is a significant lack of explaination given and the diagram appears to be slightly messy and confounding. For example what is the user written here. Is it a class or the user operating the program.
|
|
||
| ### Compare Function<br/> | ||
| #### System Architecture of the Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
Here is there supposed to be a chronological order to the diagram that I haven't noticed.?
If not then the 2 users and 2 timetables confound the reader.
| * To ensure that the users do not enter duplicate event that has the same timing or in between the time | ||
| that is entered. | ||
| * A verification check is added to the add command of the version 2.0 of WhereGotTime. | ||
|
|
There was a problem hiding this comment.
You could perhaps add a segment to explain the above sequence diagram?
|
|
||
| #### Sequence Diagram | ||
|  | ||
|
|
There was a problem hiding this comment.
Opt should be used instead of alt here as there are no alternatives provided.
* master: updated DG updated DG Bug Fix to Add command
Brach yeapcl documentation
| @@ -31,4 +174,27 @@ | |||
|
|
|||
| ## Instructions for manual testing | |||
There was a problem hiding this comment.
Is it better to follow the format given in here?
# Conflicts: # docs/DeveloperGuide.md # docs/images/Overall Architecture.png # docs/team/Overall Architecture.png # docs/team/OverallArchi.png
Updated DG
wangwaynesg
left a comment
wangwaynesg
left a comment
There was a problem hiding this comment.
Very nice developers guide! It was hard to find things to comment about!
| and location) to the timetable class into its arraylist divided by the day of the week<br/> | ||
|
|
||
| #### Sequence Design of Add Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
Perhaps this sequence diagram should be done using an online application instead of hand drawn?
| * UserList: The Class that creates an ArrayList where the User objects will be stored | ||
|
|
||
| #### Class Diagram | ||
|  |
There was a problem hiding this comment.
Perhaps some methods like getters should be omitted from the class diagram to simplify and increase readability of the class diagram? 😄
TanJunHong
left a comment
TanJunHong
left a comment
There was a problem hiding this comment.
Overall, it looks good!
There might be some mistakes here and there.
|
|
||
| The following Sequence Diagram displays how components interact when the user inputs `clear /fri` | ||
|
|
||
|  |
There was a problem hiding this comment.
This image is not loading for me. Perhaps the ".png" is supposed to be ".PNG" instead?
| and location) to the timetable class into its arraylist divided by the day of the week<br/> | ||
|
|
||
| #### Sequence Design of Add Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
Would it be better to use a software to draw the sequence diagram instead of hand drawing it?
|
|
||
| #### Design of the Delete Function<br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
Is there a better way to show 3 arrows pointing to Ui? Perhaps a slightly larger diagram would help?
|
|
||
| ### Clear Function | ||
|
|
||
| #### System Architecture of the Clear Function<br/> |
There was a problem hiding this comment.
Perhaps the system architecture of the delete and clear functions could be consolidated, since they are similar in nature?
| * UserList: An arraylist of User<br/> | ||
|
|
||
| #### Design of the Compare Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
Is there a missing bracket in the getTimetable function?
| #### Design of the Add Function<br/> | ||
|
|
||
| UML Class Diagram of the Add function: | ||
| <br/> |
There was a problem hiding this comment.
Is the bracket of the execute function supposed to be beside "execute" instead of "void"?
|  | ||
|
|
||
| The Architecture Diagram given above explains the high-level design of the list command.<br/> | ||
| * UI: The User Interface of the app<br/> | ||
| * ListCommand: The main logic command of the list function<br/> | ||
| * Timetable: The arraylist where events that are added are stored accordingly<br/> | ||
| * WhereGotTime: The main logic component of the app<br/> | ||
|
|
||
| #### Design of the List Function<br/> | ||
|
|
||
|  |
There was a problem hiding this comment.
|
|
||
|  |
There was a problem hiding this comment.
| #### Sequence Design of List Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
| #### Sequence Design of Find Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
|
|
||
| #### Sequence Diagram | ||
|  | ||
|
|
There was a problem hiding this comment.
Opt should be used instead of alt here as there are no alternatives provided.
| #### Sequence Design of Compare Function<br/> | ||
|  <br/> |
There was a problem hiding this comment.
You can improve this by maybe making the arrow that indicate returning from the Ui class from the end of the activation bar, it should reach the activation bar of the execute() in CompareCommand class. Also, try to use puml for diagrams as it would be neater compared to drawing it by hand.
| #### Sequence Design of List Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
Might be better to add an activation bar under the Ui class when printList is called. After calling the printList method, arrows can then be used to indicate returning from Ui to ListCommand and also from execute() to the ListCommand class itself
| and location) to the timetable class into its arraylist divided by the day of the week<br/> | ||
|
|
||
| #### Sequence Design of Add Function<br/> | ||
| <br/> |
There was a problem hiding this comment.
There should be a return arrow from the activation bar under the Timetable class to the activation bar of execute().
* 'master' of https://github.com/AY2021S1-CS2113-T13-3/tp: updated UG Deleted unused line in ClearCommand Updated UG - renamed heading Updated DG - included sequence diagrams for delete, clear, list and find functions Updated UG - included design for delete, clear, list and find functions Updated UG - created UML Diagrams for clear, delete, find and list commands Update DG - included system architecture for delete, clear, list and find functions Updated DG 'Architecture' section Changed 'Duke' to 'WhereGotTime' Updated 'Setting Up' section in DG Created folder for DG images
…ws an exeption to remind users to follow proper format login username /password.
Updated UG, javadoc and logging
…into tammy-testing
Organised files
Added Logging
Updated About Us
further documentation update
Minor updates
updated JAR link to 2.1
Updated JAR link
…into branch-yeapcl-Documentation * 'master' of https://github.com/AY2021S1-CS2113-T13-3/tp: Updated JAR link Deleted temporary storage folder Updated About Us Fixed checkstyle Organised files Merge branch 'master' of https://github.com/AY2021S1-CS2113-T13-3/tp into tammy-testing Added test for compare command Added logging for list, clear, find and delete functions Update UserGuide.md Update manuelmanuntag96.md
DG formatting fix
13 participants
An app that detects time table clashes