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

DG Feedback #276

Open
ongaaron96 opened this issue Apr 4, 2020 · 2 comments
Open

DG Feedback #276

ongaaron96 opened this issue Apr 4, 2020 · 2 comments
Labels
type.FeedbackFromAaron Feedback from our lovely tutor Aaron

Comments

@ongaaron96
Copy link

  • Update Figure 3
  • Update Figure 6
  • Figure 7: Include arrow heads for association labels
  • Update Figure 8

Undo/redo

  • Make this diagram bigger:
    image
  • Undo/redo Design Considerations: Include alternatives, if not theres nothing to consider

Calendar

  • Calendar Sequence Diagram: CalendarResultDisplayPane's activation bar should not be cut off after calling new CalendarPane(logic)
    image
  • Calendar "Selecting a highlighted date on the Calendar": Dont have to include example result images, just explain the internal implementation (with diagrams if possible)

Coupon Archiving

  • Figure 9. Coupon Archiving Class Diagram: has and contains labels are trivial, if its an association it already means that; can remove empty attribute/method boxes.
  • Figure 10 error:
    image
  • Figure 11: Alternative path include "[else]"; what is <<class>> above :Usage?
  • Up/Down Command History Design Considerations: include alternatives

Coupon Reminder

  • Coupon Reminder Class Diagram: same as Figure 9 and no need use; why in message and in list?
    image
  • Coupon Reminder Class structure of RemindDate field: Colours abit dark making the texts hard to read
  • RemindCommand Activity Diagram: wrong usage of branch node:
    image
  • Reminder pop up Sequence Diagram: Theres at least 5 errors here:
    image
  • Self invocation should have new activation bar. Not necessary to list out all the methods anyway
    image

Savings

  • Last Class Diagram include arrows in association labels
  • UsedCommand Activity Diagram: Wrong usage of branch node:
    image
  • What is <<class>>?
    image

Use Case

  • Can include "STASH prompts user to confirm edits" between steps 2 and 3
    image
  • Same for some other use cases. Show the interaction between user and system.
  • All user commands should have a reaction from the system, even if its something trivial like "added successfully", to provide the user with feedback

Overall comments

  • Include Figure number for all UML diagrams
  • Be consistent with the styling (font, size, arrow types, box types, figure size) and colours of all the UML diagrams (e.g. same colour scheme used to represent Logic/Model components; same style for activity diagrams)
  • For design considerations, after listing pros and cons, include a short paragraph explaining the reasons for the chosen implementation. Address the cons of the chosen implementation and the pros of the alternatives.
  • UML Diagrams are used to complement your explanation. Make sure you explain your implementation in words as well, before you display your Sequence Diagrams.
  • In your explanations, if possible, you can add in non-UML diagrams (like in undo/redo) to help readers understand better.
  • Remember to code out the keywords and be consistent
  • Overall good job, diagrams are suitable and not much mistakes :)
@alcen
Copy link

alcen commented Apr 10, 2020

@ongaaron96 The <<class>> is from textbook? I thought it is used to show static methods
image

@ongaaron96
Copy link
Author

ah my bad i didnt know that, ignore those comments! 👍

@alcen alcen added the type.FeedbackFromAaron Feedback from our lovely tutor Aaron label Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type.FeedbackFromAaron Feedback from our lovely tutor Aaron
Projects
None yet
Development

No branches or pull requests

2 participants