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 predicates in Model & Improve the test structure #233

Merged
merged 15 commits into from
Nov 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/main/java/trackitnus/commons/core/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ public class Messages {
public static final String MESSAGE_INVALID_TAB_VALUE = "Invalid tab values.";
public static final String MESSAGE_CONTACTS_LISTED_OVERVIEW = "%1$d contacts listed!";
public static final String MESSAGE_LESSON_DOES_NOT_EXIST = "There is no such lesson";
public static final String MESSAGE_TASKS_LISTED_OVERVIEW = "%1$d tasks listed!";
public static final String MESSAGE_INVALID_LESSON_DISPLAYED_INDEX = "The lesson index provided is invalid";

public static final String MESSAGE_INVALID_MODULE_DISPLAYED_INDEX = "The module index provided is invalid";
public static final String MESSAGE_MODULE_DOES_NOT_EXIST = "The module specified in the command doesn't exist.";
public static final String MESSAGE_TASK_DOES_NOT_EXIST = "There is no such task";

Expand All @@ -25,4 +23,8 @@ public class Messages {
public static final String MESSAGE_NOT_EDITED = "At least one field to edit must be provided.";
public static final String MESSAGE_DUPLICATE_MODULE = "This module already exists";
public static final String MESSAGE_DUPLICATE_TASK = "This task already exists";
public static final String MESSAGE_INVALID_INDEX = "Index is not a non-zero unsigned integer.";
public static final String DATE_MESSAGE_CONSTRAINTS = "Date should be in the format dd/MM/yyyy";
public static final String WEIGHTAGE_MESSAGE_CONSTRAINTS = "Weightage should be in the"
+ " form of a floating point number";
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ public boolean equals(Object other) {
&& editTaskDescriptor.equals(e.editTaskDescriptor);
}

@Override
public String toString() {
return "EditTaskCommand{" +
"index=" + index +
", editTaskDescriptor=" + editTaskDescriptor +
'}';
}

/**
* Stores the details to edit the task with. Each non-empty field value will replace the
* corresponding field value of the task.
Expand Down Expand Up @@ -183,7 +191,7 @@ public Optional<String> getRemark() {
}

public void setRemark(String remark) {
this.remark = remark;
this.remark = (remark == null ? "" : remark);
this.isRemarkChanged = true;
}

Expand Down Expand Up @@ -215,5 +223,15 @@ public boolean equals(Object other) {
&& getDate().equals(e.getDate())
&& getRemark().equals(e.getRemark());
}

@Override
public String toString() {
return "EditTaskDescriptor{" +
"name=" + name +
", date=" + date +
", code=" + code +
", remark='" + remark +
'}';
}
}
}
41 changes: 32 additions & 9 deletions src/main/java/trackitnus/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

import java.time.LocalDate;
import java.time.LocalTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.util.Collection;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;

import trackitnus.commons.core.Messages;
import trackitnus.commons.core.index.Index;
import trackitnus.commons.util.StringUtil;
import trackitnus.logic.parser.exceptions.ParseException;
Expand All @@ -23,14 +25,13 @@
import trackitnus.model.lesson.LessonDateTime;
import trackitnus.model.lesson.Type;
import trackitnus.model.tag.Tag;
import trackitnus.model.task.Task;

/**
* Contains utility methods used for parsing strings in the various *Parser classes.
*/
public class ParserUtil {

public static final String MESSAGE_INVALID_INDEX = "Index is not a non-zero unsigned integer.";
public static final DateTimeFormatter DATE_PATTERN = DateTimeFormatter.ofPattern("dd/MM/yyyy");

/**
* Parses {@code oneBasedIndex} into an {@code Index} and returns it. Leading and trailing whitespaces will be
Expand All @@ -41,7 +42,7 @@ public class ParserUtil {
public static Index parseIndex(String oneBasedIndex) throws ParseException {
String trimmedIndex = oneBasedIndex.trim();
if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) {
throw new ParseException(MESSAGE_INVALID_INDEX);
throw new ParseException(Messages.MESSAGE_INVALID_INDEX);
}
return Index.fromOneBased(Integer.parseInt(trimmedIndex));
}
Expand Down Expand Up @@ -198,9 +199,26 @@ public static LocalDate parseDate(String date) throws ParseException {
requireNonNull(date);
String trimmedDate = date.trim();
try {
return LocalDate.parse(trimmedDate, Task.FORMATTER);
return LocalDate.parse(trimmedDate, DATE_PATTERN);
} catch (DateTimeParseException e) {
throw new ParseException(Task.DATE_MESSAGE_CONSTRAINTS);
throw new ParseException(Messages.DATE_MESSAGE_CONSTRAINTS);
}
}

/**
* Parses a VALID {@code String date} into a {@code LocalDate}.
* Leading and trailing whitespaces will be trimmed.
* Crash the program if the date cannot be parsed (it's the caller responsibility to make sure the date passed in
* is valid!). The function is written so that the caller doesn't need to handle the possible exception since
* it's expected that no exception will be thrown.
*/
public static LocalDate parseValidDate(String date) {
requireNonNull(date);
String trimmedDate = date.trim();
try {
return LocalDate.parse(trimmedDate, DATE_PATTERN);
} catch (DateTimeParseException e) {
throw new IllegalArgumentException("An invalid date has been passed into parseValidDate");
}
}

Expand All @@ -216,7 +234,7 @@ public static double parseWeightage(String weightage) throws ParseException {
try {
return Double.parseDouble(trimmedWeightage);
} catch (NumberFormatException e) {
throw new ParseException(Task.WEIGHTAGE_MESSAGE_CONSTRAINTS);
throw new ParseException(Messages.WEIGHTAGE_MESSAGE_CONSTRAINTS);
}
}

Expand All @@ -242,17 +260,22 @@ public static String parseRemark(Optional<String> remark) {
*/
public static Type parseType(String type) throws ParseException {
requireNonNull(type);
String rawType = type.trim();
String rawType = type.trim().toLowerCase();
switch (rawType) {
case "lecture":
case "lec":
return Type.LEC;
case "tutorial":
case "tut":
return Type.TUT;
case "lab":
case "laboratory":
return Type.LAB;
case "recitation":
case "rec":
return Type.REC;
case "sectional":
case "sec":
return Type.SEC;
default:
throw new ParseException(Lesson.TYPE_MESSAGE_CONSTRAINTS);
Expand Down Expand Up @@ -284,7 +307,7 @@ private static DayOfWeek parseLessonWeekday(String weekday) throws ParseExceptio
case "sat":
return DayOfWeek.Sat;
default:
throw new ParseException(Lesson.DATE_MESSAGE_CONSTRAINTS);
throw new ParseException(Lesson.LESSON_TIME_MESSAGE_CONSTRAINTS);
}
}

Expand All @@ -311,7 +334,7 @@ public static LessonDateTime parseLessonDateTime(String date) throws ParseExcept
if (e instanceof ParseException) {
throw e;
} else {
throw new ParseException(Lesson.DATE_MESSAGE_CONSTRAINTS);
throw new ParseException(Lesson.LESSON_TIME_MESSAGE_CONSTRAINTS);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ public AddLessonCommand parse(String args) throws ParseException {
ArgumentTokenizer.tokenize(args,
PREFIX_CODE, PREFIX_TYPE, PREFIX_DATE);

if (!arePrefixesPresent(argMultimap, PREFIX_CODE, PREFIX_TYPE, PREFIX_DATE)
|| !argMultimap.getPreamble().isEmpty()) {
if (!arePrefixesPresent(argMultimap, PREFIX_CODE, PREFIX_TYPE, PREFIX_DATE)) {

Choose a reason for hiding this comment

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

Why did you remove !argMultimap.getPreamble().isEmpty()?

Copy link
Author

Choose a reason for hiding this comment

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

because argMultimap.getPreamble() is actually to check if index is present, which is unnecessary in this case (in fact this is leftovers of copying from AB3 commands)

throw new ParseException(String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT,
AddLessonCommand.MESSAGE_USAGE));
}
Expand Down
29 changes: 14 additions & 15 deletions src/main/java/trackitnus/model/ModelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,19 @@
import trackitnus.model.commons.Code;
import trackitnus.model.commons.Name;
import trackitnus.model.contact.Contact;
import trackitnus.model.contact.ContactHasTagPredicate;
import trackitnus.model.lesson.DayOfWeek;
import trackitnus.model.lesson.Lesson;
import trackitnus.model.lesson.LessonHasCodePredicate;
import trackitnus.model.lesson.LessonOnWeekdayPredicate;
import trackitnus.model.lesson.Type;
import trackitnus.model.module.Module;
import trackitnus.model.tag.Tag;
import trackitnus.model.task.Task;
import trackitnus.model.task.TaskAfterDatePredicate;
import trackitnus.model.task.TaskHasCodePredicate;
import trackitnus.model.task.TaskIsOverduePredicate;
import trackitnus.model.task.TaskOnDatePredicate;

/**
* Represents the in-memory model of the address book data.
Expand Down Expand Up @@ -302,14 +309,12 @@ public ObservableList<Lesson> getDayUpcomingLessons(LocalDate date) {
sortLesson();
updateFilteredLessonList(PREDICATE_SHOW_ALL_LESSONS);
DayOfWeek weekday = DayOfWeek.getLessonWeekDay(date);
Predicate<Lesson> predicate = lesson -> (lesson.getWeekday().equals(weekday));
return getFilteredLessonList().filtered(predicate);
return getFilteredLessonList().filtered(new LessonOnWeekdayPredicate(weekday));
}

@Override
public ObservableList<Lesson> getModuleLessons(Code code) {
Predicate<Lesson> predicate = lesson -> (lesson.getCode().equals(code));
updateFilteredLessonList(predicate);
updateFilteredLessonList(new LessonHasCodePredicate(code));
return getFilteredLessonList();
}

Expand All @@ -326,39 +331,33 @@ public Index getLessonIndex(Lesson lesson) throws CommandException {
@Override
public ObservableList<Contact> getModuleContacts(Code code) {
Tag target = new Tag(code.toString());
Predicate<Contact> predicate = contact -> (contact.getTags().contains(target));
updateFilteredContactList(predicate);
updateFilteredContactList(new ContactHasTagPredicate(target));
return getFilteredContactList();
}

@Override
public ObservableList<Task> getModuleTasks(Code code) {
Predicate<Task> p = task -> task.belongsToModule(code);
updateFilteredTaskList(p);
updateFilteredTaskList(new TaskHasCodePredicate(code));
return getFilteredTaskList();
}

@Override
public ObservableList<Task> getOverdueTasks() {
updateFilteredTaskList(Model.PREDICATE_SHOW_ALL_TASKS);
LocalDate today = LocalDate.now();
Predicate<Task> p = task -> task.getDate().isBefore(today);
return getFilteredTaskList().filtered(p);
return getFilteredTaskList().filtered(new TaskIsOverduePredicate());
}

@Override
public ObservableList<Task> getFutureTasks() {
updateFilteredTaskList(Model.PREDICATE_SHOW_ALL_TASKS);
LocalDate oneWeekLater = LocalDate.now().plusWeeks(1);
Predicate<Task> p = task -> task.getDate().isAfter(oneWeekLater);
return getFilteredTaskList().filtered(p);
return getFilteredTaskList().filtered(new TaskAfterDatePredicate(oneWeekLater));
}

@Override
public ObservableList<Task> getDayUpcomingTasks(LocalDate date) {
updateFilteredTaskList(Model.PREDICATE_SHOW_ALL_TASKS);
Predicate<Task> p = task -> task.getDate().equals(date);
return getFilteredTaskList().filtered(p);
return getFilteredTaskList().filtered(new TaskOnDatePredicate(date));
}

@Override
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/trackitnus/model/contact/ContactHasTagPredicate.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package trackitnus.model.contact;

import java.util.function.Predicate;

import trackitnus.model.tag.Tag;

public class ContactHasTagPredicate implements Predicate<Contact> {

private final Tag tag;

public ContactHasTagPredicate(Tag tag) {
this.tag = tag;
}

@Override
public boolean test(Contact contact) {
return contact.getTags().contains(tag);
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof ContactHasTagPredicate // instanceof handles nulls
&& tag.equals(((ContactHasTagPredicate) other).tag)); // state check
}
}
4 changes: 2 additions & 2 deletions src/main/java/trackitnus/model/lesson/Lesson.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public class Lesson {
public static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("d/MM/yyyy");
public static final String TIME_MESSAGE_CONSTRAINTS =
"Starting time should be earlier than finishing time";
public static final String DATE_MESSAGE_CONSTRAINTS =
"Date should be in the format \"ddd H:mm-H:mm\" (in 24-hour format), e.g. Mon 8:00-13:00";
public static final String LESSON_TIME_MESSAGE_CONSTRAINTS =
"Lesson's time should be in the format \"ddd H:mm-H:mm\" (in 24-hour format), e.g. Mon 8:00-13:00";
public static final String WEIGHTAGE_MESSAGE_CONSTRAINTS =
"Weightage should be in the form of a floating point number";
public static final String TYPE_MESSAGE_CONSTRAINTS =
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/trackitnus/model/lesson/LessonHasCodePredicate.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package trackitnus.model.lesson;

import java.util.function.Predicate;

import trackitnus.model.commons.Code;

public class LessonHasCodePredicate implements Predicate<Lesson> {

private final Code code;

public LessonHasCodePredicate(Code code) {
this.code = code;
}

@Override
public boolean test(Lesson lesson) {
return lesson.getCode().equals(code);
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof LessonHasCodePredicate // instanceof handles nulls
&& code.equals(((LessonHasCodePredicate) other).code)); // state check
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package trackitnus.model.lesson;

import java.util.function.Predicate;

public class LessonOnWeekdayPredicate implements Predicate<Lesson> {

private final DayOfWeek weekday;

public LessonOnWeekdayPredicate(DayOfWeek weekday) {
this.weekday = weekday;
}

@Override
public boolean test(Lesson lesson) {
return lesson.getWeekday().equals(weekday);
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof LessonOnWeekdayPredicate // instanceof handles nulls
&& weekday.equals(((LessonOnWeekdayPredicate) other).weekday)); // state check
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
*/
public class DuplicateLessonException extends RuntimeException {
public DuplicateLessonException() {
super("Operation would result in duplicate contacts");
super("Operation would result in duplicate lesson");
}
}
5 changes: 0 additions & 5 deletions src/main/java/trackitnus/model/task/Task.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package trackitnus.model.task;

import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
import java.util.Objects;
import java.util.Optional;

Expand All @@ -16,10 +15,6 @@
*/
public class Task {
public static final String TYPE = "T";
public static final String DATE_MESSAGE_CONSTRAINTS = "Date should be in the format dd/MM/yyyy or dd/MM/yyyy hh:mm";
public static final String WEIGHTAGE_MESSAGE_CONSTRAINTS = "Weightage should be in the"
+ " form of a floating point number";
public static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("dd/MM/yyyy");

private final Name name;
private final LocalDate date;
Expand Down