From d5a5ebbdc35e02cb34f070ff8640be563cdeb415 Mon Sep 17 00:00:00 2001 From: Clement Foo Date: Thu, 27 Oct 2022 22:50:10 +0800 Subject: [PATCH 1/2] Fix time conflict bugs Prevent user from planning item that overflows past midnight Added item past midnight error message. Start time will now reset if there is a time conflict when planning. Free now works properly with midnight reflected as 00:00 (next day). Period and Item has time getString methods since midnight of the previous day is LocalTime.Max but shown as 00:00 as a string. --- .../seedu/waddle/commons/core/Messages.java | 15 +++--- .../java/seedu/waddle/model/item/Day.java | 50 ++++++++++++------- .../java/seedu/waddle/model/item/Item.java | 15 +++--- .../waddle/model/item/exceptions/Period.java | 12 +++++ .../waddle/model/itinerary/Itinerary.java | 9 +++- 5 files changed, 71 insertions(+), 30 deletions(-) diff --git a/src/main/java/seedu/waddle/commons/core/Messages.java b/src/main/java/seedu/waddle/commons/core/Messages.java index 3e77c7f8ddb..d3c6f6e73cf 100644 --- a/src/main/java/seedu/waddle/commons/core/Messages.java +++ b/src/main/java/seedu/waddle/commons/core/Messages.java @@ -5,17 +5,20 @@ */ public class Messages { - public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command"; - public static final String MESSAGE_COPY_ERROR = "Unable to copy to clipboard"; - public static final String MESSAGE_UNAVAILABLE_COMMAND_HOME = "Command is unavailable in the home page"; - public static final String MESSAGE_UNAVAILABLE_COMMAND_ITINERARY = "Command is unavailable in the itinerary page"; + public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command."; + public static final String MESSAGE_COPY_ERROR = "Unable to copy to clipboard."; + public static final String MESSAGE_UNAVAILABLE_COMMAND_HOME = "Command is unavailable in the home page."; + public static final String MESSAGE_UNAVAILABLE_COMMAND_ITINERARY = "Command is unavailable in the itinerary page."; public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s"; - public static final String MESSAGE_INVALID_ITINERARY_DISPLAYED_INDEX = "The itinerary index provided is invalid"; - public static final String MESSAGE_INVALID_ITEM_DISPLAYED_INDEX = "The item index provided is invalid"; + public static final String MESSAGE_INVALID_ITINERARY_DISPLAYED_INDEX = "The itinerary index provided is invalid."; + public static final String MESSAGE_INVALID_ITEM_DISPLAYED_INDEX = "The item index provided is invalid."; public static final String MESSAGE_ITINERARIES_LISTED_OVERVIEW = "%1$d itineraries listed!"; public static final String MESSAGE_INVALID_STAGE = "The stage you provided is invalid! \n%1$s"; public static final String MESSAGE_CONFLICTING_ITEMS = "Quack, there is a time clash!" + "\nThe provided time clashes with:\n%1$sPlease change the start time and/or the duration."; + public static final String MESSAGE_ITEM_PAST_MIDNIGHT = + "%1$s extends past midnight which is not currently supported.\n" + + "Please split %1$s into 2 parts and plan the second part at the start of the next day."; // not meant for users to see public static final String MESSAGE_UNKNOWN_STAGE = "Unknown stage, something went wrong with the StateManager."; } diff --git a/src/main/java/seedu/waddle/model/item/Day.java b/src/main/java/seedu/waddle/model/item/Day.java index 6ba50913b7d..8c91f36fc7d 100644 --- a/src/main/java/seedu/waddle/model/item/Day.java +++ b/src/main/java/seedu/waddle/model/item/Day.java @@ -1,11 +1,14 @@ package seedu.waddle.model.item; +import static seedu.waddle.commons.core.Messages.MESSAGE_CONFLICTING_ITEMS; +import static seedu.waddle.commons.core.Messages.MESSAGE_ITEM_PAST_MIDNIGHT; + import java.time.LocalTime; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Optional; -import seedu.waddle.commons.core.Messages; import seedu.waddle.commons.core.index.Index; import seedu.waddle.logic.PdfFieldInfo; import seedu.waddle.logic.PdfFiller; @@ -43,14 +46,17 @@ public Day(int dayNumber) { * @throws CommandException Conflicting items message thrown if there are time conflicts. */ public void addItem(Item item) throws CommandException { - ArrayList conflictingItems = getConflictingItems(item); + Optional> conflictingItems = getConflictingItems(item); + if (conflictingItems.isEmpty()) { + throw new CommandException(String.format(MESSAGE_ITEM_PAST_MIDNIGHT, item.getDescription())); + } StringBuilder conflicts = new StringBuilder(); - if (!conflictingItems.isEmpty()) { - for (Item cItem : conflictingItems) { + if (!conflictingItems.get().isEmpty()) { + for (Item cItem : conflictingItems.get()) { conflicts.append(" ").append(cItem.getDescription()).append(": ").append(cItem.getStartTime()) .append(" - ").append(cItem.getEndTime()).append("\n"); } - throw new CommandException(String.format(Messages.MESSAGE_CONFLICTING_ITEMS, conflicts)); + throw new CommandException(String.format(MESSAGE_CONFLICTING_ITEMS, conflicts)); } this.itemList.add(item); this.itemList.sort(startTimeComparator); @@ -85,28 +91,38 @@ public UniqueItemList deleteDay() { } /** - * For a given item, return a list of items that conflict in time. + * For a given item, return an Optional list of items that conflict in time. + * An Optional with an empty list is returned if there are no conflicts. + * If the item goes past midnight (not allowed), an empty Optional is returned. + * If there are conflicting items, an Optional with the list of conflicting items are returned. * * @param newItem The item to check for. * @return A list of conflicting items, possibly an empty list. */ - private ArrayList getConflictingItems(Item newItem) { + private Optional> getConflictingItems(Item newItem) { ArrayList conflictingItems = new ArrayList<>(); + // item goes past midnight and overflows + if (newItem.getEndTime().isBefore(newItem.getStartTime())) { + return Optional.empty(); + } + // check for conflicting items for (Item item : this.itemList) { // same start time boolean sameStartTime = item.getStartTime().equals(newItem.getStartTime()); - // start time of new item is within the duration of a preceding item - boolean startTimeConflict = newItem.getStartTime().isAfter(item.getStartTime()) - && newItem.getStartTime().isBefore(item.getEndTime()); - // end time of new item eats into a proceeding item - boolean endTimeConflict = newItem.getEndTime().isAfter(item.getStartTime()) - && newItem.getEndTime().isBefore(item.getEndTime()); + // if new start time is before item start time + // conflict if new end time is after item start time + boolean startTimeConflict = newItem.getStartTime().isBefore(item.getStartTime()) + && newItem.getEndTime().isAfter(item.getStartTime()); + // if new start time is after item start time + // conflict if new start time is before item end time + boolean endTimeConflict = newItem.getStartTime().isAfter(item.getStartTime()) + && newItem.getStartTime().isBefore(item.getEndTime()); if (sameStartTime || startTimeConflict || endTimeConflict) { conflictingItems.add(item); } } - return conflictingItems; + return Optional.of(conflictingItems); } public int getItemSize() { @@ -138,7 +154,7 @@ public String getVacantSlots() { vacantSlots.append((this.dayNumber + 1)).append(":").append(System.getProperty("line.separator")); ArrayList vacantPeriods = new ArrayList<>(); - Period toBeSplit = new Period(LocalTime.MIN, LocalTime.parse("23:59")); + Period toBeSplit = new Period(LocalTime.MIN, LocalTime.MAX); for (Item item : this.itemList) { vacantPeriods.addAll(splitTimeSlot(toBeSplit, new Period(item.getStartTime(), item.getEndTime()))); if (vacantPeriods.size() > 0) { @@ -154,8 +170,8 @@ public String getVacantSlots() { vacantPeriods.add(toBeSplit); } for (Period period : vacantPeriods) { - vacantSlots.append(" ").append(period.getStart()).append(" - ") - .append(period.getEnd()).append(System.getProperty("line.separator")); + vacantSlots.append(" ").append(period.getStartString()).append(" - ") + .append(period.getEndString()).append(System.getProperty("line.separator")); } return vacantSlots.toString(); diff --git a/src/main/java/seedu/waddle/model/item/Item.java b/src/main/java/seedu/waddle/model/item/Item.java index bade8afe21d..b04c221772b 100644 --- a/src/main/java/seedu/waddle/model/item/Item.java +++ b/src/main/java/seedu/waddle/model/item/Item.java @@ -72,18 +72,21 @@ public void setStartTime(LocalTime startTime) { } public LocalTime getEndTime() { - LocalTime end = this.startTime.plusMinutes(this.duration.getDuration()); - // if the time overflows to next day (including 00:00), set to 23:59 - if (end.isBefore(this.startTime) || end.equals(LocalTime.MIDNIGHT)) { - return LocalTime.parse("23:59"); + LocalTime endTime = this.startTime.plusMinutes(this.duration.getDuration()); + if (this.startTime.isBefore(LocalTime.MAX) && endTime.equals(LocalTime.MIDNIGHT)) { + return LocalTime.MAX; } - return end; + return endTime; } public String getTimeString(int indents) { if (this.startTime != null) { + String endTime = getEndTime().toString(); + if (getEndTime().equals(LocalTime.MAX)) { + endTime = LocalTime.MIDNIGHT.toString(); + } if (this.duration != null) { - return Text.indent("Time: " + this.startTime + " - " + getEndTime(), indents); + return Text.indent("Time: " + this.startTime + " - " + endTime, indents); } else { return Text.indent("Time: " + this.startTime, indents); } diff --git a/src/main/java/seedu/waddle/model/item/exceptions/Period.java b/src/main/java/seedu/waddle/model/item/exceptions/Period.java index b8b4c859244..f604429aaf2 100644 --- a/src/main/java/seedu/waddle/model/item/exceptions/Period.java +++ b/src/main/java/seedu/waddle/model/item/exceptions/Period.java @@ -1,5 +1,6 @@ package seedu.waddle.model.item.exceptions; +import java.time.LocalDate; import java.time.LocalTime; /** @@ -28,4 +29,15 @@ public LocalTime getStart() { public LocalTime getEnd() { return this.end; } + + public String getStartString() { + return this.start.toString(); + } + + public String getEndString() { + if (this.end.equals(LocalTime.MAX)) { + return LocalTime.MIDNIGHT.toString() + " (next day)"; + } + return this.end.toString(); + } } diff --git a/src/main/java/seedu/waddle/model/itinerary/Itinerary.java b/src/main/java/seedu/waddle/model/itinerary/Itinerary.java index 433c8ad1b0d..e4847d0dec6 100644 --- a/src/main/java/seedu/waddle/model/itinerary/Itinerary.java +++ b/src/main/java/seedu/waddle/model/itinerary/Itinerary.java @@ -196,7 +196,14 @@ public Item planItem(Index itemIndex, DayNumber dayNumber, LocalTime startTime) Item item = this.unscheduledItemList.get(itemIndex.getZeroBased()); item.setStartTime(startTime); Day day = this.days.get(dayNumber.dayNumber.getZeroBased()); - day.addItem(item); + try { + day.addItem(item); + } catch (CommandException e) { + // if time conflict detected, reset the time of the item + item.resetStartTime(); + throw e; + } + this.unscheduledItemList.remove(itemIndex.getZeroBased()); this.budget.updateSpending(item.getCost().getValue()); return item; From fb6f764f12c11c6c775961ad9d73e516e7cafe99 Mon Sep 17 00:00:00 2001 From: Clement Foo Date: Thu, 27 Oct 2022 22:58:08 +0800 Subject: [PATCH 2/2] Fix Checkstyle --- src/main/java/seedu/waddle/commons/core/Messages.java | 4 ++-- .../java/seedu/waddle/model/item/exceptions/Period.java | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/seedu/waddle/commons/core/Messages.java b/src/main/java/seedu/waddle/commons/core/Messages.java index d3c6f6e73cf..c5cad241142 100644 --- a/src/main/java/seedu/waddle/commons/core/Messages.java +++ b/src/main/java/seedu/waddle/commons/core/Messages.java @@ -17,8 +17,8 @@ public class Messages { public static final String MESSAGE_CONFLICTING_ITEMS = "Quack, there is a time clash!" + "\nThe provided time clashes with:\n%1$sPlease change the start time and/or the duration."; public static final String MESSAGE_ITEM_PAST_MIDNIGHT = - "%1$s extends past midnight which is not currently supported.\n" + - "Please split %1$s into 2 parts and plan the second part at the start of the next day."; + "%1$s extends past midnight which is not currently supported.\n" + + "Please split %1$s into 2 parts and plan the second part at the start of the next day."; // not meant for users to see public static final String MESSAGE_UNKNOWN_STAGE = "Unknown stage, something went wrong with the StateManager."; } diff --git a/src/main/java/seedu/waddle/model/item/exceptions/Period.java b/src/main/java/seedu/waddle/model/item/exceptions/Period.java index f604429aaf2..6dfc7e2d42c 100644 --- a/src/main/java/seedu/waddle/model/item/exceptions/Period.java +++ b/src/main/java/seedu/waddle/model/item/exceptions/Period.java @@ -1,20 +1,19 @@ package seedu.waddle.model.item.exceptions; -import java.time.LocalDate; import java.time.LocalTime; /** * This class encapsulates a time period. */ public class Period { - private LocalTime start; - private LocalTime end; + private final LocalTime start; + private final LocalTime end; /** * Constructor. * * @param start Start time. - * @param end End time. + * @param end End time. */ public Period(LocalTime start, LocalTime end) { //assert(end.isAfter(start) || start.equals(end)) : "start and end time must be valid";