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

Modify dateOfLastVisit to use LocalDate instead of String #171

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 28 additions & 34 deletions src/main/java/seedu/address/model/person/DateOfLastVisit.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,26 @@
import static java.util.Objects.requireNonNull;
import static seedu.address.commons.util.AppUtil.checkArgument;


import java.time.DateTimeException;
import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
import java.time.format.ResolverStyle;

/**
* Represents a Person's last visited date by the social worker in the address book.
* Guarantees: immutable; is valid as declared in {@link #isValidDateOfLastVisit(String)}
*/
public class DateOfLastVisit implements Comparable<DateOfLastVisit> {

public static final String MESSAGE_CONSTRAINTS = "Date of last visit should be in dd-MM-yyyy format.";

// THIS VALIDATION REGEX WAS GENERATED BY CHATGPT - ACKNOWLEDGE IN DG
// date must be in the format dd-MM-yyyy
public static final String VALIDATION_REGEX =
"^((0[1-9]|[12][0-9]|3[01])-(01|03|05|07|08|10|12)-\\d{4})|"
+ "((0[1-9]|[12][0-9]|30)-(04|06|09|11)-\\d{4})|"
+ "((0[1-9]|1[0-9]|2[0-8])-02-\\d{4})|"
+ "(29-02-(?:(?:[0-9]{2}(?:0[48]|[2468][048]|[13579][26]))|(?:[0-9]{2}00)))$";
// 31st day of months with 31 days
// 30th day of months with 30 days
// 01-29 for February (ignores leap year handling)
// Days 01-31 for other months
// -------------------
public static final String MESSAGE_CONSTRAINTS = "Date of last visit should be in dd-MM-yyyy format.\n"
+ "Ensure that the month is 01-12 and the date is not later than today. ";
Copy link
Collaborator

@AbdulrahmanAlRammah AbdulrahmanAlRammah Nov 1, 2024

Choose a reason for hiding this comment

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

I don't quite understand the ensure month is 01-12 comment. Do you mean ensure that month is after day?

Edit: Oh I think you mean ensure the month is typed with 2 digits, but I think you can take this as a given since we requested dd-MM-yyyy format. You can say "Date of Last Visit should be in exactly dd-MM-yyyy format. Also it is good to add an example in the error message.

Copy link
Author

Choose a reason for hiding this comment

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

yup that was the intention and i will update the error message as suggested


public static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("dd-MM-uuuu")
.withResolverStyle(ResolverStyle.STRICT);

public final String value;

private final String[] dayMonthYear;
private final LocalDate localDate;

/**
* Constructs a {@code DateOfLastVisit}.
Expand All @@ -39,37 +33,37 @@ public DateOfLastVisit(String date) {
requireNonNull(date);
checkArgument(isValidDateOfLastVisit(date), MESSAGE_CONSTRAINTS);
value = date;
String[] init = {date.substring(0, 2), date.substring(3, 5), date.substring(6)};
this.dayMonthYear = init;
localDate = LocalDate.parse(date, DATE_TIME_FORMATTER);
}

/**
* Returns true if a given string is a valid date of last visit.
*/
public static boolean isValidDateOfLastVisit(String test) {
return test.matches(VALIDATION_REGEX);
public static boolean isValidDateOfLastVisit(String date) {
try {
LocalDate.parse(date, DATE_TIME_FORMATTER);
} catch (DateTimeException e) {
return false;
}
return isTodayOrEarlierThanToday(date);
}

/**
* Returns true if date is today or earlier than today's date.
*/
public static boolean isTodayOrEarlierThanToday(String date) {
LocalDate today = LocalDate.now();
return today.isEqual(LocalDate.parse(date, DATE_TIME_FORMATTER))
|| today.isAfter(LocalDate.parse(date, DATE_TIME_FORMATTER));
}

/**
* Compares two dates of last visit by year then month then day to find
* the right order.
*
* @param other the date to be compared to.
* @return an integer which specifies which date is earlier or if they are equal.
*/
@Override
public int compareTo(DateOfLastVisit other) {
int yearComparison = this.dayMonthYear[2].compareTo(other.dayMonthYear[2]);
if (yearComparison == 0) {
int monthComparsion = this.dayMonthYear[1].compareTo(other.dayMonthYear[1]);
if (monthComparsion == 0) {
return this.dayMonthYear[0].compareTo(other.dayMonthYear[0]);
} else {
return monthComparsion;
}
} else {
return yearComparison;
}
return localDate.compareTo(other.localDate);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package seedu.address.model.person;

import java.time.LocalDate;
import java.util.Comparator;

import seedu.address.logic.commands.exceptions.CommandException;
Expand All @@ -12,7 +13,7 @@ public class PersonComparator {
public static final String NAME = "name";
public static final String DATE_OF_LAST_VISIT = "date of last visit";
public static final String EARLIEST_VALID_DATE = "01-01-0001";
public static final String LATEST_VALID_DATE = "31-12-9999";
public static final String LATEST_VALID_DATE = LocalDate.now().format(DateOfLastVisit.DATE_TIME_FORMATTER);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It may be better to not change this. The LATEST_VALID_DATE is meant to distinguish entries with no date of last visit from those that do have. I believe in this case if you set the date of last visit to be today for one of the entries and then sort the entry with dolv as today will be mixed with those without date of last visit. Hence, it is better to keep the value as is or to set it as today + 1.

Copy link
Author

Choose a reason for hiding this comment

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

DateOfLastVisit has been updated to accept a date no later than today, which means that a date of "31-12-9999" would not be possible. I have updated the comparators for date of last visit to not use EARLIEST_VALID_DATE and LATEST_VALID_DATE. Although this makes the code slightly longer and less clean, the functionality works as expected.

private static final String SORT_EXCEPTION = "The specified parameter is invalid.";

public PersonComparator() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"email" : "[email protected]",
"address" : "wall street",
"tags" : [ ],
"dateOfLastVisit" : "01-01-2025",
"dateOfLastVisit" : "01-04-2024",
"emergencyContact" : "",
"remark" : ""
}, {
Expand All @@ -33,7 +33,7 @@
"email" : "[email protected]",
"address" : "10th street",
"tags" : [ "friends" ],
"dateOfLastVisit" : "02-01-2024",
"dateOfLastVisit" : "02-04-2024",
"emergencyContact" : "",
"remark" : ""
}, {
Expand All @@ -42,7 +42,7 @@
"email" : "[email protected]",
"address" : "michegan ave",
"tags" : [ ],
"dateOfLastVisit" : "10-10-2024",
"dateOfLastVisit" : "10-07-2024",
"emergencyContact" : "",
"remark" : ""
}, {
Expand All @@ -60,7 +60,7 @@
"email" : "[email protected]",
"address" : "4th street",
"tags" : [ ],
"dateOfLastVisit" : "05-06-2024",
"dateOfLastVisit" : "05-09-2024",
"emergencyContact" : "",
"remark" : ""
} ]
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/seedu/address/logic/commands/SortCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,30 @@ public class SortCommandTest {

@Test
public void execute_nameAscendingOrder_success() {
executeSuccessfulSortTest(PersonComparator.NAME, true, modelAscendingDateOfLastVisit);
rayray39 marked this conversation as resolved.
Show resolved Hide resolved
executeSuccessfulSortTest(PersonComparator.NAME, true, modelAscendingName);
}

@Test
public void execute_nameDescendingOrder_success() {
executeSuccessfulSortTest(PersonComparator.NAME, false, modelDescendingDateOfLastVisit);
executeSuccessfulSortTest(PersonComparator.NAME, false, modelDescendingName);
}

@Test
public void execute_dateOfLastVisitAscending_success() {
executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, true, modelDescendingName);
executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, true, modelAscendingDateOfLastVisit);
}

@Test
public void execute_dateOfLastVisitDescending_success() {
executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, false, modelAscendingName);
executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, false, modelDescendingDateOfLastVisit);
}

private void executeSuccessfulSortTest(String sortParameter, boolean isAscending, Model model) {
rayray39 marked this conversation as resolved.
Show resolved Hide resolved
SortCommand sortCommand = new SortCommand(sortParameter, isAscending);
String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
sortParameter, isAscending ? "ascending" : "descending");
Model expectedModel = new ModelManager(getTypicalAddressBook(sortParameter, isAscending), new UserPrefs());
assertCommandSuccess(sortCommand, modelDescendingName, expectedMessage, expectedModel);
assertCommandSuccess(sortCommand, model, expectedMessage, expectedModel);
}

@Test
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/seedu/address/model/person/DateOfLastVisitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static seedu.address.testutil.Assert.assertThrows;

import java.time.LocalDate;

import org.junit.jupiter.api.Test;


public class DateOfLastVisitTest {

@Test
Expand Down Expand Up @@ -36,6 +39,14 @@ public void isValidDateOfLastVisit() {
assertFalse(DateOfLastVisit.isValidDateOfLastVisit("31-04-2024")); // 31st of even month
assertFalse(DateOfLastVisit.isValidDateOfLastVisit("29-02-2023")); // 29th feb of non leap year

// date later than today
String tomorrow = LocalDate.now().plusDays(1).format(DateOfLastVisit.DATE_TIME_FORMATTER);
assertFalse(DateOfLastVisit.isValidDateOfLastVisit(tomorrow)); // date is later than today

// date is today
String today = LocalDate.now().format(DateOfLastVisit.DATE_TIME_FORMATTER);
assertTrue(DateOfLastVisit.isValidDateOfLastVisit(today)); // date is equal to today

// valid dateOfLastVisits
assertTrue(DateOfLastVisit.isValidDateOfLastVisit("01-01-0000")); // year 0000
assertTrue(DateOfLastVisit.isValidDateOfLastVisit("31-01-2024")); // month with 31st
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/seedu/address/testutil/TypicalPersons.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ public class TypicalPersons {
.withTags("owesMoney", "friends").withDateOfLastVisit("01-02-2024")
.withEmergencyContact("92173902").build();
public static final Person CARL = new PersonBuilder().withName("Carl Kurz").withPhone("95352563")
.withEmail("[email protected]").withAddress("wall street").withDateOfLastVisit("01-01-2025")
.withEmail("[email protected]").withAddress("wall street").withDateOfLastVisit("01-04-2024")
.withEmergencyContact().build();
public static final Person DANIEL = new PersonBuilder().withName("Daniel Meier").withPhone("87652533")
.withEmail("[email protected]").withAddress("10th street").withTags("friends")
.withDateOfLastVisit("02-01-2024").withEmergencyContact().build();
.withDateOfLastVisit("02-04-2024").withEmergencyContact().build();
public static final Person ELLE = new PersonBuilder().withName("Elle Meyer").withPhone("94822249")
.withEmail("[email protected]").withAddress("michegan ave").withDateOfLastVisit("10-10-2024")
.withEmail("[email protected]").withAddress("michegan ave").withDateOfLastVisit("10-07-2024")
.withEmergencyContact().build();
public static final Person FIONA = new PersonBuilder().withName("Fiona Kunz").withPhone("94824270")
.withEmail("[email protected]").withAddress("little tokyo").withDateOfLastVisit("23-08-2024")
.withEmergencyContact().build();
public static final Person GEORGE = new PersonBuilder().withName("George Best").withPhone("94824421")
.withEmail("[email protected]").withAddress("4th street").withDateOfLastVisit("05-06-2024")
.withEmail("[email protected]").withAddress("4th street").withDateOfLastVisit("05-09-2024")
.withEmergencyContact().build();

// Manually added
Expand Down Expand Up @@ -127,10 +127,10 @@ public static List<Person> getTypicalPersonsDescendingName() {
}

public static List<Person> getTypicalPersonsAscendingDateOfLastVisit() {
return new ArrayList<>(Arrays.asList(ALICE, DANIEL, BENSON, GEORGE, FIONA, ELLE, CARL));
return new ArrayList<>(Arrays.asList(ALICE, BENSON, CARL, DANIEL, ELLE, FIONA, GEORGE));
rayray39 marked this conversation as resolved.
Show resolved Hide resolved
}

public static List<Person> getTypicalPersonsDescendingDateOfLastVisit() {
return new ArrayList<>(Arrays.asList(CARL, ELLE, FIONA, GEORGE, BENSON, DANIEL, ALICE));
return new ArrayList<>(Arrays.asList(GEORGE, FIONA, ELLE, DANIEL, CARL, BENSON, ALICE));
}
}