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

Refactor phone to deadline in project #68

Conversation

TCQian
Copy link

@TCQian TCQian commented Oct 4, 2020

No description provided.

@TCQian TCQian requested a review from lll-jy October 4, 2020 05:22
@TCQian TCQian linked an issue Oct 4, 2020 that may be closed by this pull request
@@ -326,7 +326,7 @@ Use case ends.
**MSS:**

1. Team leader create a new team member profile.
2. TMTS asks for the details of the team member such as name, phone number and email address.
2. TMTS asks for the details of the team member such as name, deadline number and email address.
Copy link

Choose a reason for hiding this comment

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

deadline number may be changed? e.g. deadline datetime

@@ -351,7 +351,7 @@ Use case ends.
1. Team leader chooses to edit a team member's profile.
2. PTS asks for the name of the team member whose profile is to be edited.
3. Team leader keys in the name of the team member.
4. TMTS asks for the new information of team member such as name, phone number and email address.
4. TMTS asks for the new information of team member such as name, deadline number and email address.
Copy link

Choose a reason for hiding this comment

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

Similar as above

@@ -93,7 +93,7 @@ In `src/test/data/`, data meant for testing purposes are stored. While keeping t
{
"projects": [ {
"name": "Project with invalid name field: Ha!ns Mu@ster",
"phone": "9482424",
"deadline": "9482424",
Copy link

Choose a reason for hiding this comment

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

This may need to be changed?

@@ -0,0 +1,103 @@
package seedu.address.model.project;
Copy link

Choose a reason for hiding this comment

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

I can see that all your code works. But would it be easier if we use java LocalDate and LocalDateTime to deal with it? And it may be easier for us in the future work if we want to filter projects by deadline.

import seedu.address.model.project.Email;
import seedu.address.model.project.Name;
import seedu.address.model.project.Phone;
Copy link

Choose a reason for hiding this comment

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

All the examples may need to updated according to the validation regex.

@@ -1,12 +1,12 @@
{
Copy link

Choose a reason for hiding this comment

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

This file may need to be changed (for deadline regex)

@@ -4,9 +4,9 @@
import static seedu.address.commons.core.Messages.MESSAGE_INVALID_PROJECT_DISPLAYED_INDEX;
import static seedu.address.commons.core.Messages.MESSAGE_UNKNOWN_COMMAND;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.DEADLINE_DESC_A;
Copy link

Choose a reason for hiding this comment

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

I think we'd better keep the format aligned, say use DEADLINE_DESC_AMY as the name. And we can change AMY to project names together for future refactoring work.

@@ -43,8 +43,8 @@

public static final String NAME_DESC_AMY = " " + PREFIX_NAME + VALID_NAME_AMY;
public static final String NAME_DESC_BOB = " " + PREFIX_NAME + VALID_NAME_BOB;
public static final String PHONE_DESC_AMY = " " + PREFIX_PHONE + VALID_PHONE_AMY;
public static final String PHONE_DESC_BOB = " " + PREFIX_PHONE + VALID_PHONE_BOB;
public static final String DEADLINE_DESC_A = " " + PREFIX_DEADLINE + VALID_DEADLINE_A;
Copy link

Choose a reason for hiding this comment

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

Same as the above comments about naming

@@ -2,10 +2,10 @@

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static seedu.address.logic.commands.CommandTestUtil.DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.DESC_A;
Copy link

Choose a reason for hiding this comment

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

Same as above comments about naming.

@TCQian
Copy link
Author

TCQian commented Oct 5, 2020

Opened a new branch for this, will close this PR and migrate to another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor Attributes Of Project: Phone
2 participants