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
[CS2113T-T09-1] ConTech #24
base: master
Are you sure you want to change the base?
[CS2113T-T09-1] ConTech #24
Changes from 250 commits
18fcaa5
56f11c0
8bd5717
a98fae8
e8aa334
ee3bc07
0f28962
63f2f5d
56652e6
17a07c2
2289404
d7240b0
26c4392
f07e452
d466817
ef6fb13
094dfca
58eb2f9
01445d5
6479e3b
e173806
dde4267
3826eff
f7e62d6
9115d98
f83b9ef
dba382d
30957af
0ba3982
f1b0c07
b3a7a35
a5bd01e
17172c8
9bd4fe0
78ce064
3116895
b580226
aa753fb
46aac4d
c6fc10e
3f73558
382caac
a971169
7920ffe
28c88f0
3e511e4
dd92516
71dd867
7863337
eb8775c
79cb9e5
0af56b4
7af4d1e
e54fbfb
735426e
b232862
a2c65a0
cadd4df
8dfdfc1
dfa2182
152dd29
6557b8a
2774f8e
4478e7a
91f5de9
01f2a32
24254d9
44b416f
efbf827
81813e8
d819792
89bb68c
bb0e15b
c6195c7
28e0cf0
7416f26
2a6ae7c
bf9db8f
7169e9a
159f614
31d71c1
e4f5523
0749b4e
617852b
b4d8c93
9c09929
381d6c9
431370c
28ada49
d726e5f
a44110c
de98b73
cc3a245
a96558b
c55beb6
9f3a8bb
c13b252
ad83d6a
5e0cb00
db1ff0a
c44147b
06b4312
a798aed
1d9cfd2
d2d6af2
301ae1b
504c6d8
fe61793
8b36498
b3fae31
4bb17d2
4581dd3
5f16b8a
b391877
ad40beb
4478393
cb43817
11a5d48
7bab42a
e2573bc
1e864a7
0b9e5c0
dc7d4c9
b70197a
7ca0061
f134696
708df4b
a2348a9
352f882
1c0e454
4f20c1a
1aa3539
1970f07
f356bba
b52e2ba
65c5371
b85f0ac
3f5d55e
a8bf949
37c6caa
16ca0e2
1a6b635
11596bc
c1940d8
5efc2f6
bbffa33
92c1cf1
cd1ae03
cd115fe
530717e
a13c86f
78a4988
7a0ef26
937a862
f5e3868
1211a54
e814308
f6c4273
115ddcf
d7b2008
f04130c
35fdc96
e4f7b40
c87b34d
1846fdf
1bd9f7a
610d55c
bc05c30
8c5571a
7741a48
f9da9ff
be3a0af
68230e8
876520f
653a898
2049ccb
f3d6011
52c4be3
52d0ed5
ee307ee
be732d8
693008f
6ef1cef
1b2bd93
4678ad1
5563cf3
fe565b5
54f69e2
dff0dc2
fdd50c8
6edbe7f
c6399b9
6b6cc30
3edd3dc
4a4d20e
e38e1db
7601757
b5d8ef1
c31c7e1
7c6d354
ae30c85
06e8c3f
66e2073
1868de2
3700b2c
2e7b2fc
a1cfecd
ad3088e
ccc7a62
5613cd7
210ad1e
d728bbf
f31ab04
9fa2585
5cd8cda
215580c
f89d665
dc29324
9622535
a022c8c
4ee56ce
4d4bea2
5ef88e7
183dc22
6ab0938
118fdd9
c88b9d4
1483c6e
e354ffe
1fb2935
d2a2846
63a1a1c
762e5f4
692ffbf
08dac1c
0044343
22feab5
fe6f2d5
145dfc3
eb1dffa
18c34e4
e85f9df
dc4b41f
b106517
a23a14d
f94f319
b83daac
09f3fd9
8b45a00
18192ba
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.
Text-ui does not seem to be complete, along with Command and ContactList, maybe can try finish up the details 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.
Sections under “Design” seems not finished. Include “Command”, “ContactList”, and “TextUi”. Please fill them in 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.
For the Command section can consider add in the Class diagram to relate all the commands together
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.
Please remember to update the "Command" and "ContactList" sections 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.
i think for each functionality, adding some sample inputs & outputs is much clear?
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.
This diagram is a bit too big and contains a bit too many details. It would be hard to view this in the FDF format. If all the implementation is necessary for the illustration, the loop in this diagram maybe can be placed in a separate diagram as reference.
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.
I think the constructor for EditContactCommand have not been 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.
ya, I agree with Mick. The diagram seems to be too big which causes the words inside the diagram a bit hard to see. You can try changing the
maxMessageLength
of this diagram. Also you can consider puttingnew
before aConstructor()
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.
same issue: diagram too big, can consider representing the circled part to something simpler
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.
One way to solve this is by putting reference frame :D
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 SearchContactCommand constructor have also not been called 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.
Diagram also quite big, summarise the diagram more, especially the green part
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 you could provide some instructions here for the manual testing?
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 can include some manual testing instructions?