Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[CS2113-F12-2] EquipmentManager #21
base: master
Are you sure you want to change the base?
[CS2113-F12-2] EquipmentManager #21
Changes from 168 commits
7e8fced
57f061c
0734b17
ab4e53f
ec57a81
01975b8
1bbef72
18acfc3
05c6e37
1b93be4
15a19c6
061cdd9
43fd0ef
8c8d59f
c575044
629ca22
ffc71c5
cc12f64
24a23c7
106e543
be99f2a
c7fce04
81f1b31
a6f9cfb
19ffe09
37e38a8
1d77861
b3ec295
103d600
49b427c
40aedfb
74fbb68
da0c19b
2f8cdb5
5c5d02b
628bb76
a256b4a
1d8c444
068000d
a37eac0
33e1561
3b1973b
15a8567
ab30bb6
4b007a3
575ffc2
5c1944f
d9a2260
15083e6
d012a98
9aaa4a5
4ce2fee
48091c4
744ca10
1b01955
7daddaa
598b834
aff1a94
c899522
09ec2e0
c92851b
4cc3cc9
74fb528
f83b455
aeea70e
193e0cd
725c901
e7f8d80
78044ed
0871a56
a25a9bd
a8bf95f
7a9b035
73f9e26
bfc8d1f
9b921fc
b6c4fd4
1e25993
79516b1
ab76a1c
56f2662
9ffdb1d
914275d
408c3a0
7639822
5fe7255
e2a5908
472d189
e81593f
73b9822
f5c7547
92f3686
2748802
0266e64
b643499
1c6d7e9
699f82b
fcb2c0d
f4b3884
97449e5
83ab598
61085ef
76ade6d
2557729
9e0b760
7393dbd
5c47bf4
a37648a
2d6500a
941f6aa
f3ef396
70c9576
3fad737
6e0cd55
04f3dc7
f57d3aa
0bf0196
11dce75
95e5c71
d113b1a
15ab1e1
4d2813a
bc5c8b9
e7b19e6
cfb2132
2ecb645
ebba1c5
7c66485
b26539c
77f00bf
dff0711
6279221
a31a215
338f29a
a36ec57
64fec6c
ef4927f
e64321d
b7c685c
f94108f
4567f8c
0f0d7cf
028fa80
daa63e7
927f724
ee03f62
e39f380
3c1bcf3
fffeae2
7357caf
6c90b2e
19fa96c
fc74fb3
2bf2c9e
5907a09
c26200f
b0a3244
b883a13
309230c
45d9520
fc54558
0468964
4325f44
d89d102
8966f2e
d16e468
4aa6a78
75a3275
27e0955
ec27609
84da80f
f31eed4
0bf50e9
9564b60
3af7ab4
2d41b8e
a21807e
042dc64
979beb5
4640b93
ab7e932
5b6db61
6453c2d
909760a
ee99055
86bc0da
1bbb2e7
91cdfbe
75a5ede
c29e241
01e05dd
80215ec
f6f7f35
57b2a76
6b2bef0
b7a889f
7578d41
689d706
8debc8e
7837794
ef1c6c0
5f7883d
87d11a3
92ecb83
7913e08
dd421a9
6f354d6
431ad04
98074c1
2eb11c7
633745f
ac45f66
8ef249a
2c0cdb2
baab98c
9ade160
7b3cef3
b1df158
d9677f8
c0769db
3199793
6a0e8a2
1e1b00e
7d252b1
905863e
4f1790c
6d6d954
208785a
b3e1e5b
721288e
e932e3a
ec269c9
995ff01
36b34cf
20f0798
55ec940
afdf5e2
2a67c2d
4ba8bf1
1a6744c
1e14ac4
4111a77
c162645
2e0651c
d0c0540
1ee66be
6fcd2ad
71d165a
dad297a
d3ea1f4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it's better to have an Architecture Diagram to aid understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As others suggested, class diagrams or an architecture diagram would be appreciated here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A class diagram could be appropriate here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A visual diagram of Parser may make this section easier to understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there's some mistyping for the last one? It's the same as the second one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider giving an example usage for the add, delete and update operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For alternative paths, perhaps it is an optional path. If my understanding is correct, you don't have an else statement in your code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the start of Parser activation bar be before the method is called?
Should the return of the Parser class be at the end of the activation bar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a return of control after calling prepareModification()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a deactivation in the activation bar after returning from parser() call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the return arrow be at the end of the activation bar and the solid arrow be at the start of the activation bar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the calling arrow should touch the very top of the activation bar on the right, and the return arrow should come from the very bottom. This would signify that updateEquipment begins its lifecycle and isSuccessfulIUpdate ends it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no return arrow when execute command is being called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the calling and returning arrows touch the very tops and bottoms of the activation bars?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Parser making a new command object here? If Parser is making a new command object, it may be the diagram should signify a constructor call (by having the calling arrow touch the "command: UpdateCommand" box and have an activation bar immediately following that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sequence diagram is a bit too big (in resolution). Placing this diagram in the final submission would be hard to be viewed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using class or object diagram to illustrate how equipment manager works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explanations of each of these features may be beneficial
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding the steps of the implementation of EquipmentManager as per the Parser Component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Product scope is necessary for the DG. Please try to finish this part soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can definitely have more user stories.