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.

Sign up for GitHub

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

Jump to bottom

[W5.11][F10-3] Ong Wee Keong #177

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

[W5.11][F10-3] Ong Wee Keong #177

wants to merge 21 commits into from

Conversation

ongweekeong
Copy link

Edited restrictions to 'add' command:

  1. Persons with same name AND same phone number/email cannot be added to the address book.
  2. Persons with different name but same phone number AND email cannot be added to the address book.
  3. Persons with same name but different phone number AND email can be added to the address book.

iamputradanish and others added 21 commits September 6, 2018 14:39
@iamputradanish
Added confirm/cancel clear option to after clear command
fe9fe0f
@iamputradanish
Merge pull request #1 from CS2113-AY1819S1-F10-3/confirm_cancel_clear…
66db9a8 
…_command

Added confirm/cancel clear option to after clear command
@iamputradanish
Update UserGuide
f22df67 
Includes Clear/Cancel command prompt guide
@iamputradanish
Merge pull request #2 from CS2113-AY1819S1-F10-3/iamputradanish-patch-1
ff12999 
Update UserGuide
@andyrobert3
Delete by name command
ffa8b03
@andyrobert3
Added user guide for new delete_name command
d3d0e55
@andyrobert3
Merge branch 'master' into feature/delete_name
0bdbacb
@ShreyasKp
Merge pull request #3 from CS2113-AY1819S1-F10-3/feature/delete_name
51c1830 
Add delete_name command
@ongweekeong
added nonsense
07b409a
@ongweekeong
Merge commit '51c183064febaed0dea56ca2e129f40484781a94' into tutorial-4
4971881
@andyrobert3
Merge branch 'master' into feature/delete_name
b7c6185 
Trying to merge
@andyrobert3
Merge remote-tracking branch 'origin/feature/delete_name' into W5.11
94bece0
@ongweekeong
deleted nonsense
ba96540
@andyrobert3
Merge remote-tracking branch 'origin/feature/delete_name' into W5.11
51af3c6
@muhdharun
added total command and edited user guide
2a5b474
@muhdharun
Merge branch 'master' into w5.11
a47f7a8
@ongweekeong
Merge pull request #4 from muhdharun/w5.11
03bf4b6 
added total command and edited user guide
@ongweekeong
Added further constraints to 'add' feature
871908b
@ongweekeong
Edited user guide
9eddf0d
@ongweekeong
Merge branch 'W5.11' of https://github.com/CS2113-AY1819S1-F10-3/addr…
eec6c50 
…essbook-level2 into W5.11
@ongweekeong
Added restrictions to 'add' command & removed duplicate labels
4ebc7f0
@ongweekeong ongweekeong changed the title [W5.11][CS2113-AY1819S1-F10-3] Ong Wee Keong [W5.11][F10-3] Ong Wee Keong Sep 13, 2018
fanyiii
fanyiii reviewed Sep 16, 2018
View reviewed changes
Copy link

@fanyiii fanyiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A sensible improvement for checking duplicate person. On a hindsight, will having the same phone OR email suffice as well? Also, remember to update the help command as well as the test cases to reflect the change.

src/seedu/addressbook/data/person/ReadOnlyPerson.java
&& other.getEmail().equals(this.getEmail())
|| (other.getName().equals(this.getName())
&& (other.getPhone().equals(this.getPhone())
|| other.getEmail().equals(this.getEmail()))));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement on the duplication checks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fanyiii fanyiii fanyiii left review comments

@stephlewyh stephlewyh Awaiting requested review from stephlewyh

Assignees

@fanyiii fanyiii

Labels
Reviewed team.3 tutorial.F10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ongweekeong @fanyiii @nus-se-pr-bot @iamputradanish @andyrobert3 @ShreyasKp @muhdharun