From c87980daa5edf461b6f1a0a33f265082114e00ac Mon Sep 17 00:00:00 2001 From: Sean Yap Date: Tue, 26 Mar 2019 08:21:39 +0800 Subject: [PATCH] Edit Data Classes, remove unnecessary dependencies (#120) * v1.1 * fix some errors and typos * Refactor entire project, remove all traces of Addressbook * Update developer guide for ExportCommandP (#80) * Update documentation for view command (#81) * Remove remaining Addressbook classes * Refactor some classes * fix missing files issue * Add test cases for add function, Utils and other code enhancements * fix codacy issues * fix codacy issues * Add tests for Utils * Add diagrams * Update Ui.png * fix issue with Ui.png * Update Codacy Badge link due to reinitialization. * Enhance ordering of slots printed * Update UserGuide.adoc with feedback from peers * Update UserGuide.adoc with nicer tables * Update UserGuide.adoc: fix formatting issues * UserGuide.adoc Remove potential Netlify breaking code * README.adoc revert changes * UserGuide.adoc revert Netlify breakdown test code * Add tests for Add, Edit and Delete commands and optimise code * Fix checkstyle error * Update UserGuide.adoc with Managing Slots and diagrams * Solved unchecked or unsafe operation warnings * Edit Data Classes, remove unnecessary dependencies --- src/planmysem/commands/AddCommand.java | 14 +---- src/planmysem/commands/DeleteCommand.java | 11 +--- src/planmysem/commands/EditCommand.java | 30 +++------ src/planmysem/commands/FindCommand.java | 9 ++- src/planmysem/commands/ListCommand.java | 10 +-- src/planmysem/common/Messages.java | 12 ++-- src/planmysem/common/Utils.java | 25 +++----- src/planmysem/data/Planner.java | 8 +-- .../exception/DuplicateDataException.java | 10 --- src/planmysem/data/semester/Day.java | 3 +- src/planmysem/data/semester/Semester.java | 32 +--------- src/planmysem/data/slot/Description.java | 52 ---------------- src/planmysem/data/slot/Location.java | 52 ---------------- src/planmysem/data/slot/Name.java | 58 ----------------- src/planmysem/data/slot/ReadOnlySlot.java | 13 ++-- src/planmysem/data/slot/Slot.java | 58 ++++++++--------- src/planmysem/data/tag/Tag.java | 53 ---------------- src/planmysem/parser/Parser.java | 19 ++---- src/planmysem/storage/jaxb/AdaptedDay.java | 2 +- .../storage/jaxb/AdaptedPlanner.java | 2 +- .../storage/jaxb/AdaptedSemester.java | 2 +- src/planmysem/storage/jaxb/AdaptedSlot.java | 62 ++++++++++++------- src/planmysem/storage/jaxb/AdaptedTag.java | 51 --------------- test/java/planmysem/common/UtilsTest.java | 48 -------------- test/java/planmysem/logic/LogicTest.java | 61 ++++++++---------- 25 files changed, 146 insertions(+), 551 deletions(-) delete mode 100644 src/planmysem/data/exception/DuplicateDataException.java delete mode 100644 src/planmysem/data/slot/Description.java delete mode 100644 src/planmysem/data/slot/Location.java delete mode 100644 src/planmysem/data/slot/Name.java delete mode 100644 src/planmysem/data/tag/Tag.java delete mode 100644 src/planmysem/storage/jaxb/AdaptedTag.java diff --git a/src/planmysem/commands/AddCommand.java b/src/planmysem/commands/AddCommand.java index a09d7ec46..83f4f534e 100644 --- a/src/planmysem/commands/AddCommand.java +++ b/src/planmysem/commands/AddCommand.java @@ -6,14 +6,10 @@ import java.util.Set; import java.util.TreeMap; -import planmysem.common.Utils; import planmysem.data.exception.IllegalValueException; import planmysem.data.recurrence.Recurrence; import planmysem.data.semester.Day; import planmysem.data.semester.Semester; -import planmysem.data.slot.Description; -import planmysem.data.slot.Location; -import planmysem.data.slot.Name; import planmysem.data.slot.Slot; /** @@ -39,13 +35,10 @@ public class AddCommand extends Command { /** * Convenience constructor using raw values. - * - * @throws IllegalValueException if any of the raw values are invalid */ public AddCommand(LocalDate date, String name, String location, String description, LocalTime startTime, - int duration, Set tags, Set recurrences) throws IllegalValueException { - slot = new Slot(new Name(name), new Location(location), new Description(description), - startTime, duration, Utils.parseTags(tags)); + int duration, Set tags, Set recurrences) { + slot = new Slot(name, location, description, startTime, duration, tags); recurrence = new Recurrence(recurrences, date); } @@ -56,8 +49,7 @@ public AddCommand(LocalDate date, String name, String location, String descripti */ public AddCommand(int day, String name, String location, String description, LocalTime startTime, int duration, Set tags, Set recurrences) throws IllegalValueException { - slot = new Slot(new Name(name), new Location(location), new Description(description), - startTime, duration, Utils.parseTags(tags)); + slot = new Slot(name, location, description, startTime, duration, tags); recurrence = new Recurrence(recurrences, day); } diff --git a/src/planmysem/commands/DeleteCommand.java b/src/planmysem/commands/DeleteCommand.java index 6462012b7..1500054f3 100644 --- a/src/planmysem/commands/DeleteCommand.java +++ b/src/planmysem/commands/DeleteCommand.java @@ -8,11 +8,8 @@ import javafx.util.Pair; import planmysem.common.Messages; -import planmysem.common.Utils; -import planmysem.data.exception.IllegalValueException; import planmysem.data.semester.ReadOnlyDay; import planmysem.data.slot.ReadOnlySlot; -import planmysem.data.tag.Tag; /** * Adds a person to the address book. @@ -34,15 +31,13 @@ public class DeleteCommand extends Command { public static final String MESSAGE_SUCCESS_NO_CHANGE = "No Slots were deleted.\n\n%1$s"; public static final String MESSAGE_SUCCESS = "%1$s Slots deleted.\n\n%2$s\n%3$s"; - private final Set tags = new HashSet<>(); + private final Set tags = new HashSet<>(); /** * Convenience constructor using raw values. - * - * @throws IllegalValueException if any of the raw values are invalid */ - public DeleteCommand(Set tags) throws IllegalValueException { - this.tags.addAll(Utils.parseTags(tags)); + public DeleteCommand(Set tags) { + this.tags.addAll(tags); } /** diff --git a/src/planmysem/commands/EditCommand.java b/src/planmysem/commands/EditCommand.java index d85e10146..402f2a396 100644 --- a/src/planmysem/commands/EditCommand.java +++ b/src/planmysem/commands/EditCommand.java @@ -10,11 +10,8 @@ import javafx.util.Pair; import planmysem.common.Messages; -import planmysem.common.Utils; -import planmysem.data.exception.IllegalValueException; import planmysem.data.semester.ReadOnlyDay; import planmysem.data.slot.ReadOnlySlot; -import planmysem.data.tag.Tag; /** * Adds a person to the address book. @@ -46,16 +43,14 @@ public class EditCommand extends Command { private final String name; private final String location; private final String description; - private final Set tags = new HashSet<>(); - private final Set newTags = new HashSet<>(); + private final Set tags = new HashSet<>(); + private final Set newTags = new HashSet<>(); /** * Convenience constructor using raw values. - * - * @throws IllegalValueException if any of the raw values are invalid */ public EditCommand(String name, LocalTime startTime, int duration, String location, String description, - Set tags, Set newTags) throws IllegalValueException { + Set tags, Set newTags) { this.date = null; this.startTime = startTime; this.duration = duration; @@ -63,10 +58,10 @@ public EditCommand(String name, LocalTime startTime, int duration, String locati this.location = location; this.description = description; if (tags != null) { - this.tags.addAll(Utils.parseTags(tags)); + this.tags.addAll(tags); } if (newTags != null) { - this.newTags.addAll(Utils.parseTags(newTags)); + this.newTags.addAll(newTags); } } @@ -74,8 +69,7 @@ public EditCommand(String name, LocalTime startTime, int duration, String locati * Convenience constructor using raw values. */ public EditCommand(int index, String name, LocalDate date, LocalTime startTime, int duration, - String location, String description, Set newTags) - throws IllegalValueException { + String location, String description, Set newTags) { super(index); this.date = date; this.startTime = startTime; @@ -84,7 +78,7 @@ public EditCommand(int index, String name, LocalDate date, LocalTime startTime, this.location = location; this.description = description; if (newTags != null) { - this.newTags.addAll(Utils.parseTags(newTags)); + this.newTags.addAll(newTags); } } @@ -112,12 +106,8 @@ public CommandResult execute() { String successMessage = craftSuccessMessage(selectedSlots); for (Map.Entry> entry : selectedSlots.entrySet()) { - try { - planner.editSlot(entry.getKey(), entry.getValue().getValue(), date, - startTime, duration, name, location, description, newTags); - } catch (IllegalValueException ive) { - return new CommandResult(MESSAGE_FAIL_ILLEGAL_VALUE); - } + planner.editSlot(entry.getKey(), entry.getValue().getValue(), date, + startTime, duration, name, location, description, newTags); } return new CommandResult(String.format(MESSAGE_SUCCESS, selectedSlots.size(), @@ -178,7 +168,7 @@ public String craftSuccessMessage(Map sb.append("\nTags: "); StringJoiner sj = new StringJoiner(", "); - for (Tag tag : newTags) { + for (String tag : newTags) { StringBuilder sb2 = new StringBuilder(); sb2.append("\""); sb2.append(tag); diff --git a/src/planmysem/commands/FindCommand.java b/src/planmysem/commands/FindCommand.java index 2dd1d5835..c89e70195 100644 --- a/src/planmysem/commands/FindCommand.java +++ b/src/planmysem/commands/FindCommand.java @@ -13,7 +13,6 @@ import planmysem.data.semester.ReadOnlyDay; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; /** @@ -46,13 +45,13 @@ public CommandResult execute() { for (Map.Entry entry : planner.getAllDays().entrySet()) { for (Slot slot : entry.getValue().getSlots()) { if (isFindByName) { - if (Pattern.matches(".*" + keyword + ".*", slot.getName().value)) { + if (Pattern.matches(".*" + keyword + ".*", slot.getName())) { selectedSlots.put(entry.getKey(), new Pair<>(entry.getValue(), slot)); } } else { - Set tagSet = slot.getTags(); - for (Tag tag : tagSet) { - if (Pattern.matches(".*" + keyword + ".*", tag.value)) { + Set tagSet = slot.getTags(); + for (String tag : tagSet) { + if (Pattern.matches(".*" + keyword + ".*", tag)) { selectedSlots.put(entry.getKey(), new Pair<>(entry.getValue(), slot)); } } diff --git a/src/planmysem/commands/ListCommand.java b/src/planmysem/commands/ListCommand.java index 9bc8b7a61..c7322a510 100644 --- a/src/planmysem/commands/ListCommand.java +++ b/src/planmysem/commands/ListCommand.java @@ -12,7 +12,6 @@ import planmysem.data.semester.ReadOnlyDay; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; /** * Displays a list of all slots in the planner whose name matches the argument keyword. @@ -38,6 +37,7 @@ public ListCommand(String name, String tag) { this.keyword = (name == null) ? tag.trim() : name.trim(); this.isListByName = (name != null); } + @Override public CommandResult execute() { Map> selectedSlots = new TreeMap<>(); @@ -45,14 +45,14 @@ public CommandResult execute() { for (Map.Entry entry : planner.getAllDays().entrySet()) { for (Slot slot : entry.getValue().getSlots()) { if (isListByName) { - if (slot.getName().value.equalsIgnoreCase(keyword)) { + if (slot.getName().equalsIgnoreCase(keyword)) { selectedSlots.put(entry.getKey(), new Pair<>(entry.getValue(), slot)); } } else { - Set tagSet = slot.getTags(); - for (Tag tag : tagSet) { + Set tagSet = slot.getTags(); + for (String tag : tagSet) { // if (slot.getTags().contains(keyword)) - if (tag.value.equalsIgnoreCase(keyword)) { + if (tag.equalsIgnoreCase(keyword)) { selectedSlots.put(entry.getKey(), new Pair<>(entry.getValue(), slot)); } } diff --git a/src/planmysem/common/Messages.java b/src/planmysem/common/Messages.java index 8ad408670..79a036666 100644 --- a/src/planmysem/common/Messages.java +++ b/src/planmysem/common/Messages.java @@ -7,7 +7,6 @@ import javafx.util.Pair; import planmysem.data.semester.ReadOnlyDay; import planmysem.data.slot.ReadOnlySlot; -import planmysem.data.tag.Tag; /** * Container for user visible messages. @@ -16,10 +15,8 @@ public class Messages { public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s"; public static final String MESSAGE_INVALID_COMMAND_FORMAT_ADDITIONAL = "Invalid command format! \n%1$s\n\n%2$s"; - public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid"; public static final String MESSAGE_INVALID_MULTIPLE_PARAMS = "Either search by NAME or by TAG only, not both."; public static final String MESSAGE_INVALID_SLOT_DISPLAYED_INDEX = "The slot index provided is invalid"; - public static final String MESSAGE_PERSON_NOT_IN_ADDRESSBOOK = "Person could not be found in address book"; public static final String MESSAGE_SLOT_NOT_IN_PLANNER = "Slot could not be found in Planner"; public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!"; public static final String MESSAGE_SLOTS_LISTED_OVERVIEW = "%1$d slots listed!"; @@ -38,19 +35,20 @@ public class Messages { + "\n\t12-Hour in the form of `hh:mm+AM|PM`. e.g. \"12:30am\"" + "\n\tOr perhaps type a duration in minutes. e.g. \"60\" to represent 60 minutes"; + public static final String MESSAGE_ILLEGAL_VALUE = "Illegal value detected!"; /** * Craft selected message. */ - public static String craftSelectedMessage(Set tags) { + public static String craftSelectedMessage(Set tags) { StringBuilder sb = new StringBuilder(); sb.append("Selected Slots containing tags: \n"); int count = 1; - for (Tag tag : tags) { + for (String tag : tags) { sb.append(count); sb.append(".\t"); - sb.append(tag.toString()); + sb.append(tag); sb.append("\n"); count++; } @@ -89,7 +87,7 @@ public static String craftListMessage(Map parseTags(Set tags) throws IllegalValueException { - if (tags == null) { - return null; - } - Set tagSet = new HashSet<>(); - for (String tag : tags) { - tagSet.add(new Tag(tag)); - } - return tagSet; - } + // public static Set parseTags(Set tags) { + // if (tags == null) { + // return null; + // } + // Set tagSet = new HashSet<>(); + // for (String tag : tags) { + // tagSet.add(new String(tag)); + // } + // return tagSet; + // } /** diff --git a/src/planmysem/data/Planner.java b/src/planmysem/data/Planner.java index 4209037d0..9aacecc01 100644 --- a/src/planmysem/data/Planner.java +++ b/src/planmysem/data/Planner.java @@ -8,13 +8,11 @@ import java.util.TreeMap; import javafx.util.Pair; -import planmysem.data.exception.IllegalValueException; import planmysem.data.semester.Day; import planmysem.data.semester.ReadOnlyDay; import planmysem.data.semester.Semester; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; /** * Represents the entire Planner. Contains the data of the Planner. @@ -64,12 +62,10 @@ public void removeSlot(LocalDate date, ReadOnlySlot slot) { /** * Edit specific slot within the planner. - * - * @throws IllegalValueException if a targetDate is not found in the semester. */ public void editSlot(LocalDate targetDate, ReadOnlySlot targetSlot, LocalDate date, LocalTime startTime, int duration, String name, String location, - String description, Set tags) throws IllegalValueException { + String description, Set tags) { semester.editSlot(targetDate, targetSlot, date, startTime, duration, name, location, description, tags); } @@ -150,7 +146,7 @@ public Day getDay(LocalDate date) { /** * gets all slots in the Planner containing all specified tags. */ - public Map> getSlots(Set tags) { + public Map> getSlots(Set tags) { final Map> selectedSlots = new TreeMap<>(); for (Map.Entry entry : getAllDays().entrySet()) { diff --git a/src/planmysem/data/exception/DuplicateDataException.java b/src/planmysem/data/exception/DuplicateDataException.java deleted file mode 100644 index 84289866d..000000000 --- a/src/planmysem/data/exception/DuplicateDataException.java +++ /dev/null @@ -1,10 +0,0 @@ -package planmysem.data.exception; - -/** - * Signals an error caused by duplicate data where there should be none. - */ -public abstract class DuplicateDataException extends IllegalValueException { - public DuplicateDataException(String message) { - super(message); - } -} diff --git a/src/planmysem/data/semester/Day.java b/src/planmysem/data/semester/Day.java index 46f8b2af4..4639e3345 100644 --- a/src/planmysem/data/semester/Day.java +++ b/src/planmysem/data/semester/Day.java @@ -5,7 +5,6 @@ import java.util.ArrayList; import java.util.Objects; -import planmysem.data.exception.IllegalValueException; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; @@ -46,7 +45,7 @@ public void addSlot(Slot slot) { * Edit a slot in the day. */ public void editSlot(ReadOnlySlot targetSlot, LocalTime startTime, int duration, - String name, String location, String description) throws IllegalValueException { + String name, String location, String description) { for (Slot slot : slots) { if (slot.equals(targetSlot)) { if (startTime != null) { diff --git a/src/planmysem/data/semester/Semester.java b/src/planmysem/data/semester/Semester.java index 97da35c74..583b6e547 100644 --- a/src/planmysem/data/semester/Semester.java +++ b/src/planmysem/data/semester/Semester.java @@ -15,11 +15,8 @@ import java.util.TreeMap; import java.util.stream.Collectors; -import planmysem.data.exception.DuplicateDataException; -import planmysem.data.exception.IllegalValueException; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; /** * A list of days. Does not allow null elements or duplicates. @@ -327,22 +324,10 @@ private static String[] getSemesterDetails(LocalDate date, HashMap getSlots(Set tags) { + public Map getSlots(Set tags) { Map selectedSlots = new TreeMap<>(); for (Map.Entry day : days.entrySet()) { @@ -378,13 +363,9 @@ public void removeSlot(LocalDate date, ReadOnlySlot slot) { /** * Edits a Slot in the Semester. - * - * @throws IllegalValueException if a targetDate is not found in the semester. */ public void editSlot(LocalDate targetDate, ReadOnlySlot targetSlot, LocalDate date, LocalTime startTime, - int duration, String name, String location, String description, Set tags) - throws IllegalValueException { - + int duration, String name, String location, String description, Set tags) { Slot editingSlot = days.get(targetDate).getSlots().stream() .filter(s -> s.equals(targetSlot)).findAny().orElse(null); @@ -542,15 +523,6 @@ public int hashCode() { recessDays, readingDays, normalDays, examDays); } - /** - * Signals that an operation would have violated the 'no duplicates' property. - */ - public static class DuplicateDayException extends DuplicateDataException { - protected DuplicateDayException() { - super("Operation would result in duplicate days"); - } - } - /** * Signals that an operation targeting a specified Day in the list would fail because * there is no such matching Day in the list. diff --git a/src/planmysem/data/slot/Description.java b/src/planmysem/data/slot/Description.java deleted file mode 100644 index 229d65feb..000000000 --- a/src/planmysem/data/slot/Description.java +++ /dev/null @@ -1,52 +0,0 @@ -package planmysem.data.slot; - -import planmysem.data.exception.IllegalValueException; - -/** - * Represents a Slot's description in the address book. - * Guarantees: immutable; is valid as declared in {@link #isValid(String)} - */ -public class Description { - private static final String MESSAGE_CONSTRAINTS = - "Slot's description should be spaces or alphanumeric characters"; - private static final String VALIDATION_REGEX = ".+"; - - public final String value; - - /** - * Validates given value. - * - * @throws IllegalValueException if given value string is invalid. - */ - public Description(String value) throws IllegalValueException { - if (value != null && !isValid(value)) { - throw new IllegalValueException(MESSAGE_CONSTRAINTS); - } - this.value = value; - } - - /** - * Returns true if a given string is a valid Slot description. - */ - public static boolean isValid(String value) { - return "".equals(value) || value.matches(VALIDATION_REGEX); - } - - @Override - public String toString() { - return value; - } - - @Override - public boolean equals(Object other) { - return other == this // short circuit if same object - || (other instanceof Description // instanceof handles nulls - && this.value.equals(((Description) other).value)); // state check - } - - @Override - public int hashCode() { - return value.hashCode(); - } - -} diff --git a/src/planmysem/data/slot/Location.java b/src/planmysem/data/slot/Location.java deleted file mode 100644 index 9f7ebe1ea..000000000 --- a/src/planmysem/data/slot/Location.java +++ /dev/null @@ -1,52 +0,0 @@ -package planmysem.data.slot; - -import planmysem.data.exception.IllegalValueException; - -/** - * Represents a Slot's location in the address book. - * Guarantees: immutable; is valid as declared in {@link #isValid(String)} - */ -public class Location { - private static final String MESSAGE_CONSTRAINTS = - "Slot's location should be spaces or alphanumeric characters"; - private static final String VALIDATION_REGEX = ".+"; - - private final String value; - - /** - * Validates given value. - * - * @throws IllegalValueException if given value string is invalid. - */ - public Location(String value) throws IllegalValueException { - if (value != null && !isValid(value)) { - throw new IllegalValueException(MESSAGE_CONSTRAINTS); - } - this.value = value; - } - - /** - * Returns true if a given string is a valid. - */ - public static boolean isValid(String value) { - return "".equals(value) || value.matches(VALIDATION_REGEX); - } - - @Override - public String toString() { - return value; - } - - @Override - public boolean equals(Object other) { - return other == this // short circuit if same object - || (other instanceof Location // instanceof handles nulls - && this.value.equals(((Location) other).value)); // state check - } - - @Override - public int hashCode() { - return value.hashCode(); - } - -} diff --git a/src/planmysem/data/slot/Name.java b/src/planmysem/data/slot/Name.java deleted file mode 100644 index f8b6405d0..000000000 --- a/src/planmysem/data/slot/Name.java +++ /dev/null @@ -1,58 +0,0 @@ -package planmysem.data.slot; - -import planmysem.data.exception.IllegalValueException; - -/** - * Represents a Slot's name in the Planner. - * Guarantees: immutable; is valid as declared in {@link #isValid(String)} - */ -public class Name { - private static final String MESSAGE_CONSTRAINTS = "Slot names should be spaces or alphanumeric characters"; - private static final String VALIDATION_REGEX = ".+"; - - public final String value; - - /** - * Validates given value. - * - * @throws IllegalValueException if given name string is invalid. - */ - public Name(String value) throws IllegalValueException { - if (!isValid(value)) { - throw new IllegalValueException(MESSAGE_CONSTRAINTS); - } - this.value = value; - } - - /** - * Returns true if a given string is a valid slot name. - */ - public static boolean isValid(String test) { - return test.matches(VALIDATION_REGEX); - } - - /** - * Retrieves a listing of every word in the name, in order. - */ - // public List getWordsInValue() { - // return Arrays.asList(value.split("\\s+")); - // } - - @Override - public String toString() { - return value; - } - - @Override - public boolean equals(Object other) { - return other == this // short circuit if same object - || (other instanceof Name // instanceof handles nulls - && this.value.equals(((Name) other).value)); // state check - } - - @Override - public int hashCode() { - return value.hashCode(); - } - -} diff --git a/src/planmysem/data/slot/ReadOnlySlot.java b/src/planmysem/data/slot/ReadOnlySlot.java index 7aa7e3a1d..b70101a8b 100644 --- a/src/planmysem/data/slot/ReadOnlySlot.java +++ b/src/planmysem/data/slot/ReadOnlySlot.java @@ -4,16 +4,15 @@ import java.util.Set; import planmysem.common.Utils; -import planmysem.data.tag.Tag; /** * A read-only immutable interface for a Slot in the Planner. * Implementations should guarantee: details are present and not null, field values are validated. */ public interface ReadOnlySlot { - Name getName(); - Location getLocation(); - Description getDescription(); + String getName(); + String getLocation(); + String getDescription(); int getDuration(); LocalTime getStartTime(); @@ -21,7 +20,7 @@ public interface ReadOnlySlot { * The returned {@code Set} is a deep copy of the internal {@code Set}, * changes on the returned list will not affect the slot's internal tags. */ - Set getTags(); + Set getTags(); /** * Returns true if the values inside this object is same as @@ -67,12 +66,12 @@ default String getAsTextShowAll() { sb.append("\nTags: "); int count = 1; - for (Tag tag : getTags()) { + for (String tag : getTags()) { sb.append("\n"); sb.append("\t"); sb.append(count); sb.append(".\t"); - sb.append(tag.toString()); + sb.append(tag); count++; } return sb.toString(); diff --git a/src/planmysem/data/slot/Slot.java b/src/planmysem/data/slot/Slot.java index e4ddceccf..7456ed19d 100644 --- a/src/planmysem/data/slot/Slot.java +++ b/src/planmysem/data/slot/Slot.java @@ -6,26 +6,24 @@ import java.util.Set; import planmysem.common.Utils; -import planmysem.data.exception.IllegalValueException; -import planmysem.data.tag.Tag; /** * Represents a slot in the planner. * Guarantees: details are present and not null, field values are validated. */ public class Slot implements ReadOnlySlot { - private final Set tags = new HashSet<>(); - private Name name; - private Location location; - private Description description; + private final Set tags = new HashSet<>(); + private String name; + private String location; + private String description; private LocalTime startTime; private int duration; /** * Assumption: Every field must be present and not null. */ - public Slot(Name name, Location location, Description description, - LocalTime startTime, LocalTime endTime, Set tags) { + public Slot(String name, String location, String description, + LocalTime startTime, LocalTime endTime, Set tags) { this.name = name; this.location = location; this.description = description; @@ -39,8 +37,8 @@ public Slot(Name name, Location location, Description description, /** * Assumption: Every field must be present and not null. */ - public Slot(Name name, Location location, Description description, - LocalTime startTime, int duration, Set tags) { + public Slot(String name, String location, String description, + LocalTime startTime, int duration, Set tags) { this.name = name; this.location = location; this.description = description; @@ -53,7 +51,6 @@ public Slot(Name name, Location location, Description description, /** * Copy constructor. - * @throws IllegalValueException if value is invalid */ public Slot(ReadOnlySlot source) { this(source.getName(), source.getLocation(), source.getDescription(), @@ -62,49 +59,41 @@ public Slot(ReadOnlySlot source) { /** * Set name. - * @throws IllegalValueException if value is invalid */ - public void setName(String value) throws IllegalValueException { + public void setName(String value) { if (value == null) { return; } - name = new Name(value); + name = value; } /** * Set location. - * @throws IllegalValueException if value is invalid */ - public void setLocation(String value) throws IllegalValueException { + public void setLocation(String value) { if (value == null) { return; } - if ("".equals(value)) { - location = new Location(null); - } else { - location = new Location(value); - } + location = value; } /** * Set description. - * @throws IllegalValueException if value is invalid */ - public void setDescription(String value) throws IllegalValueException { + public void setDescription(String value) { if (value == null) { return; } - if ("".equals(value)) { - description = new Description(null); - } else { - description = new Description(value); - } + description = value; } /** * Set start time. */ public void setStartTime(LocalTime value) { + if (value == null) { + return; + } startTime = value; } @@ -116,17 +105,17 @@ public void setDuration(int value) { } @Override - public Name getName() { + public String getName() { return name; } @Override - public Location getLocation() { + public String getLocation() { return location; } @Override - public Description getDescription() { + public String getDescription() { return description; } @@ -141,14 +130,17 @@ public LocalTime getStartTime() { } @Override - public Set getTags() { + public Set getTags() { return tags; } /** * Replaces this slot's tags with the tags in {@code replacement}. */ - public void setTags(Set tags) { + public void setTags(Set tags) { + if (tags == null) { + return; + } this.tags.clear(); this.tags.addAll(tags); } diff --git a/src/planmysem/data/tag/Tag.java b/src/planmysem/data/tag/Tag.java deleted file mode 100644 index 5bdda71d2..000000000 --- a/src/planmysem/data/tag/Tag.java +++ /dev/null @@ -1,53 +0,0 @@ -package planmysem.data.tag; - -import planmysem.data.exception.IllegalValueException; - -/** - * Represents a Tag in the Planner. - * Guarantees: immutable; name is valid as declared in {@link #isValidTagName(String)} - */ -public class Tag { - - private static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric"; - private static final String VALIDATION_REGEX = ".+"; - - public final String value; - - /** - * Validates given value. - * - * @throws IllegalValueException if the given tag name string is invalid. - */ - public Tag(String name) throws IllegalValueException { - String value = name.trim(); - if (!isValidTagName(value)) { - throw new IllegalValueException(MESSAGE_CONSTRAINTS); - } - this.value = value; - } - - /** - * Returns true if a given value is a valid. - */ - private static boolean isValidTagName(String test) { - return test.matches(VALIDATION_REGEX); - } - - @Override - public boolean equals(Object other) { - return other == this // short circuit if same object - || (other instanceof Tag // instanceof handles nulls - && this.value.equals(((Tag) other).value)); // state check - } - - @Override - public int hashCode() { - return value.hashCode(); - } - - @Override - public String toString() { - return value; - } - -} diff --git a/src/planmysem/parser/Parser.java b/src/planmysem/parser/Parser.java index 9ff033923..ce58e0dd2 100644 --- a/src/planmysem/parser/Parser.java +++ b/src/planmysem/parser/Parser.java @@ -261,26 +261,19 @@ private Command prepareEdit(String args) { } } + // The following are not mandatory and can be null String name = getFirstInSet(arguments.get(PARAMETER_NEW_NAME)); String location = getFirstInSet(arguments.get(PARAMETER_NEW_LOCATION)); String description = getFirstInSet(arguments.get(PARAMETER_NEW_DESCRIPTION)); Set newTags = arguments.get(PARAMETER_NEW_TAG); if (index == -1) { - try { - return new EditCommand(name, startTime, duration, location, description, tags, newTags); - } catch (IllegalValueException ive) { - return new IncorrectCommand(ive.getMessage()); - } + return new EditCommand(name, startTime, duration, location, description, tags, newTags); } else { String nd = getFirstInSet(arguments.get(PARAMETER_NEW_DATE)); LocalDate date = Utils.parseDate(nd); - try { - return new EditCommand(index, name, date, startTime, duration, location, description, newTags); - } catch (IllegalValueException ive) { - return new IncorrectCommand(ive.getMessage()); - } + return new EditCommand(index, name, date, startTime, duration, location, description, newTags); } } @@ -301,11 +294,7 @@ private Command prepareDelete(String args) { } if (index == -1) { - try { - return new DeleteCommand(tags); - } catch (IllegalValueException ive) { - return new IncorrectCommand(ive.getMessage()); - } + return new DeleteCommand(tags); } else { return new DeleteCommand(index); } diff --git a/src/planmysem/storage/jaxb/AdaptedDay.java b/src/planmysem/storage/jaxb/AdaptedDay.java index c430eb334..0eb517f56 100644 --- a/src/planmysem/storage/jaxb/AdaptedDay.java +++ b/src/planmysem/storage/jaxb/AdaptedDay.java @@ -64,7 +64,7 @@ public boolean isAnyRequiredFieldMissing() { /** * Converts this jaxb-friendly adapted Day object into the Day object. * - * @throws IllegalValueException if there were any data constraints violated in the adapted Day + * @throws IllegalValueException if there were any data constraints violated in the AdaptedSemester */ public Day toModelType() throws IllegalValueException { final ArrayList slots = new ArrayList<>(); diff --git a/src/planmysem/storage/jaxb/AdaptedPlanner.java b/src/planmysem/storage/jaxb/AdaptedPlanner.java index af5b12c81..ffec202dc 100644 --- a/src/planmysem/storage/jaxb/AdaptedPlanner.java +++ b/src/planmysem/storage/jaxb/AdaptedPlanner.java @@ -45,7 +45,7 @@ public boolean isAnyRequiredFieldMissing() { /** * Converts this jaxb-friendly {@code AdaptedPlanner} object into the corresponding(@code Planner} object. * - * @throws IllegalValueException if there were any data constraints violated in the adapted person + * @throws IllegalValueException if there were any data constraints violated in the AdaptedSemester */ public Planner toModelType() throws IllegalValueException { return new Planner(semester.toModelType()); diff --git a/src/planmysem/storage/jaxb/AdaptedSemester.java b/src/planmysem/storage/jaxb/AdaptedSemester.java index 1cce90d96..7e5eeb269 100644 --- a/src/planmysem/storage/jaxb/AdaptedSemester.java +++ b/src/planmysem/storage/jaxb/AdaptedSemester.java @@ -101,7 +101,7 @@ public boolean isAnyRequiredFieldMissing() { /** * Converts this jaxb-friendly adapted person object into the Person object. * - * @throws IllegalValueException if there were any data constraints violated in the adapted person + * @throws IllegalValueException if there were any data constraints violated in the AdaptedSemester */ public Semester toModelType() throws IllegalValueException { final String name = this.name; diff --git a/src/planmysem/storage/jaxb/AdaptedSlot.java b/src/planmysem/storage/jaxb/AdaptedSlot.java index dc5422bcb..50f95d9ec 100644 --- a/src/planmysem/storage/jaxb/AdaptedSlot.java +++ b/src/planmysem/storage/jaxb/AdaptedSlot.java @@ -1,6 +1,9 @@ package planmysem.storage.jaxb; +import static planmysem.common.Messages.MESSAGE_ILLEGAL_VALUE; + import java.time.LocalTime; +import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -10,12 +13,8 @@ import planmysem.common.Utils; import planmysem.data.exception.IllegalValueException; -import planmysem.data.slot.Description; -import planmysem.data.slot.Location; -import planmysem.data.slot.Name; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; /** * JAXB-friendly adapted person data holder class. @@ -32,7 +31,7 @@ public class AdaptedSlot { @XmlElement(required = true) private String startTime; @XmlElement(required = true) - private List tags = new ArrayList<>(); + private List tags = new ArrayList<>(); /** * No-arg constructor for JAXB use. @@ -46,48 +45,63 @@ public AdaptedSlot() { * @param source future changes to this will not affect the created AdaptedPerson */ public AdaptedSlot(ReadOnlySlot source) { - name = source.getName().toString(); - location = source.getLocation().toString(); - description = source.getDescription().toString(); + name = source.getName(); + location = source.getLocation(); + description = source.getDescription(); duration = source.getDuration(); startTime = source.getStartTime().toString(); tags = new ArrayList<>(); - for (Tag tag : source.getTags()) { - tags.add(new AdaptedTag(tag)); - } + tags.addAll(source.getTags()); } /** * Returns true if any required field is missing. */ public boolean isAnyRequiredFieldMissing() { - for (AdaptedTag tag : tags) { - if (tag.isAnyRequiredFieldMissing()) { + for (String tag : tags) { + if (tag.isEmpty()) { return true; } } - // second call only happens if name, location, description, time are all not null + + // second call only happens if name, duration, start time are all not null return Utils.isAnyNull(name, duration, startTime); } /** * Converts this jaxb-friendly adapted person object into the Person object. * - * @throws IllegalValueException if there were any data constraints violated in the adapted person + * @throws IllegalValueException if there were any data constraints violated in the AdaptedSemester */ public Slot toModelType() throws IllegalValueException { - final Name name = new Name(this.name); - final Location location = new Location(this.location); - final Description description = new Description(this.description); - final LocalTime startTime = LocalTime.parse(this.startTime); - final int duration = this.duration; - - final Set tags = new HashSet<>(); - for (AdaptedTag tag : this.tags) { - tags.add(tag.toModelType()); + if (hasIllegalValues(name) || hasIllegalValues(location) || hasIllegalValues(description) || duration < 0) { + throw new IllegalValueException(MESSAGE_ILLEGAL_VALUE); + } + + final LocalTime startTime; + + try { + startTime = LocalTime.parse(this.startTime); + } catch (DateTimeParseException dtpe) { + throw new IllegalValueException(MESSAGE_ILLEGAL_VALUE); + } + + final Set tags = new HashSet<>(); + for (String tag : this.tags) { + if (hasIllegalValues(tag)) { + throw new IllegalValueException(MESSAGE_ILLEGAL_VALUE); + } + tags.add(tag); } return new Slot(name, location, description, startTime, duration, tags); } + + /** + * Returns true if value has any illegal values. + */ + private static boolean hasIllegalValues(String value) { + return value.contains("/"); + } } diff --git a/src/planmysem/storage/jaxb/AdaptedTag.java b/src/planmysem/storage/jaxb/AdaptedTag.java deleted file mode 100644 index 9b39ad3cc..000000000 --- a/src/planmysem/storage/jaxb/AdaptedTag.java +++ /dev/null @@ -1,51 +0,0 @@ -package planmysem.storage.jaxb; - -import javax.xml.bind.annotation.XmlValue; - -import planmysem.common.Utils; -import planmysem.data.exception.IllegalValueException; -import planmysem.data.tag.Tag; - -/** - * JAXB-friendly adapted tag data holder class. - */ -public class AdaptedTag { - @XmlValue - private String value; - - /** - * No-arg constructor for JAXB use. - */ - public AdaptedTag() { - } - - /** - * Converts a given Tag into this class for JAXB use. - * - * @param source future changes to this will not affect the created AdaptedTag - */ - public AdaptedTag(Tag source) { - value = source.value; - } - - /** - * Returns true if any required field is missing. - *

- * JAXB does not enforce (required = true) without a given XML schema. - * Since we do most of our validation using the data class constructors, the only extra logic we need - * is to ensure that every xml element in the document is present. JAXB sets missing elements as null, - * so we check for that. - */ - public boolean isAnyRequiredFieldMissing() { - return Utils.isAnyNull(value); - } - - /** - * Converts this jaxb-friendly adapted tag object into the Tag object. - * - * @throws IllegalValueException if there were any data constraints violated in the adapted person - */ - public Tag toModelType() throws IllegalValueException { - return new Tag(value); - } -} diff --git a/test/java/planmysem/common/UtilsTest.java b/test/java/planmysem/common/UtilsTest.java index 82d34399a..e1c2c45c4 100644 --- a/test/java/planmysem/common/UtilsTest.java +++ b/test/java/planmysem/common/UtilsTest.java @@ -9,15 +9,10 @@ import java.time.LocalDate; import java.time.LocalTime; -import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.Set; import org.junit.Test; -import planmysem.data.exception.IllegalValueException; -import planmysem.data.tag.Tag; public class UtilsTest { @Test @@ -164,49 +159,6 @@ public void parse_integer_unsuccessful() { assertEquals(Utils.parseInteger("OO"), -1); } - @Test - public void parse_tags_successful() { - List listOfTag = new ArrayList<>(); - listOfTag.add("0"); - listOfTag.add("tag1"); - listOfTag.add("tag 2"); - listOfTag.add("tag 3 super long tag"); - - Set tagStrings = new HashSet<>(listOfTag); - - Set expectedTags = new HashSet<>(); - Set tags = new HashSet<>(); - try { - expectedTags = new HashSet<>(Arrays.asList(new Tag("0"), - new Tag("tag1"), new Tag("tag 2"), - new Tag("tag 3 super long tag"))); - - tags = Utils.parseTags(tagStrings); - } catch (IllegalValueException ive) { - } - - assertEquals(tags, expectedTags); - - try { - tags = Utils.parseTags(null); - } catch (IllegalValueException ive) { - } - - assertEquals(tags, null); - } - - @Test - public void parse_tags_unsuccessful() { - Set tags = new HashSet<>(); - - try { - tags = Utils.parseTags(null); - } catch (IllegalValueException ive) { - } - - assertEquals(tags, null); - } - @Test public void parse_get_duration_successful() { LocalTime startTime = LocalTime.now(); diff --git a/test/java/planmysem/logic/LogicTest.java b/test/java/planmysem/logic/LogicTest.java index 3bb1d13d0..b9c9c3549 100644 --- a/test/java/planmysem/logic/LogicTest.java +++ b/test/java/planmysem/logic/LogicTest.java @@ -43,12 +43,8 @@ import planmysem.data.semester.Day; import planmysem.data.semester.ReadOnlyDay; import planmysem.data.semester.Semester; -import planmysem.data.slot.Description; -import planmysem.data.slot.Location; -import planmysem.data.slot.Name; import planmysem.data.slot.ReadOnlySlot; import planmysem.data.slot.Slot; -import planmysem.data.tag.Tag; import planmysem.storage.StorageFile; public class LogicTest { @@ -300,20 +296,18 @@ public void execute_edit_successful() throws Exception { expectedPlanner.addSlot(dateToBeAdded, slotToBeAdded); // create tags - Set rawTags = new HashSet<>(); - rawTags.add("CS2113T"); - Set tags = Utils.parseTags(rawTags); + Set tags = new HashSet<>(); + tags.add("CS2113T"); - Set rawNewTags = new HashSet<>(); - rawNewTags.add("CS2101"); - Set newTags = Utils.parseTags(rawNewTags); + Set newTags = new HashSet<>(); + newTags.add("CS2101"); expectedPlanner.editSlot(dateToBeAdded, slotToBeAdded, null, LocalTime.of(4, 0), 60, "test", "testlo", "testdes", newTags); // Just to generate the crafted message in this case. EditCommand ec = new EditCommand("test", LocalTime.of(4, 0), 60, - "testlo", "testdes", rawTags, rawNewTags); + "testlo", "testdes", tags, newTags); // execute command and verify result assertCommandBehavior("edit t/CS2113T nt/CS2101 nn/test nl/testlo ndes/testdes nst/04:00 net/60", @@ -326,10 +320,8 @@ public void execute_edit_successful() throws Exception { @Test public void execute_edit_no_change_successful() throws Exception { - TestDataHelper helper = new TestDataHelper(); - Set rawTags = new HashSet<>(); - rawTags.add("someTagThatDoesNotExist"); - Set tags = Utils.parseTags(rawTags); + Set tags = new HashSet<>(); + tags.add("someTagThatDoesNotExist"); assertCommandBehavior("edit t/someTagThatDoesNotExist n/test", String.format(EditCommand.MESSAGE_SUCCESS_NO_CHANGE, @@ -391,9 +383,8 @@ public void execute_delete_successful() throws Exception { @Test public void execute_delete_no_change_successful() throws Exception { TestDataHelper helper = new TestDataHelper(); - Set rawTags = new HashSet<>(); - rawTags.add("someTagThatDoesNotExist"); - Set tags = Utils.parseTags(rawTags); + Set tags = new HashSet<>(); + tags.add("someTagThatDoesNotExist"); assertCommandBehavior(helper.generateDeleteCommand(tags), String.format(DeleteCommand.MESSAGE_SUCCESS_NO_CHANGE, @@ -474,14 +465,12 @@ private void assertCommandBehavior(String inputCommand, class TestDataHelper{ Slot slotOne() throws Exception { - Name name = new Name("CS2113T Tutorial"); - Location location = new Location("COM2 04-11"); - Description description = new Description("Topic: Sequence Diagram"); + String name = "CS2113T Tutorial"; + String location = "COM2 04-11"; + String description = "Topic: Sequence Diagram"; LocalTime startTime = LocalTime.parse("08:00"); LocalTime endTime = LocalTime.parse("09:00"); - Tag tag1 = new Tag("CS2113T"); - Tag tag2 = new Tag("Tutorial"); - Set tags = new HashSet<>(Arrays.asList(tag1, tag2)); + Set tags = new HashSet<>(Arrays.asList( "CS2113T", "Tutorial")); return new Slot(name, location, description, startTime, endTime, tags); } @@ -494,12 +483,12 @@ Slot slotOne() throws Exception { */ Slot generateSlot(int seed) throws Exception { return new Slot( - new Name("slot " + seed), - new Location("location " + Math.abs(seed)), - new Description("description " + Math.abs(seed)), + "slot " + seed, + "location " + Math.abs(seed), + "description " + Math.abs(seed), LocalTime.parse("00:00"), LocalTime.parse("00:00"), - new HashSet<>(Arrays.asList(new Tag("tag" + Math.abs(seed)), new Tag("tag" + Math.abs(seed + 1)))) + new HashSet<>(Arrays.asList("tag" + Math.abs(seed), "tag" + Math.abs(seed + 1))) ); } @@ -520,9 +509,9 @@ String generateAddCommand(Slot s, LocalDate date, String recurrence) { cmd.add("des/" + s.getDescription()); } - Set tags = s.getTags(); + Set tags = s.getTags(); if (tags != null) { - for(Tag tag : tags){ + for(String tag : tags){ cmd.add("t/" + tag); } } @@ -549,9 +538,9 @@ String generateAddCommand(Slot s, int day, String recurrence) { cmd.add("des/" + s.getDescription()); } - Set tags = s.getTags(); + Set tags = s.getTags(); if (tags != null) { - for(Tag tag : tags){ + for(String tag : tags){ cmd.add("t/" + tag); } } @@ -562,13 +551,13 @@ String generateAddCommand(Slot s, int day, String recurrence) { } /** Generates the correct delete command based on tags */ - String generateDeleteCommand(Set tags) { + String generateDeleteCommand(Set tags) { StringJoiner cmd = new StringJoiner(" "); cmd.add("delete"); if (tags != null) { - for(Tag tag : tags){ + for(String tag : tags){ cmd.add("t/" + tag); } } @@ -582,9 +571,9 @@ String generateDeleteCommand(Slot slot) { cmd.add("delete"); - Set tags = slot.getTags(); + Set tags = slot.getTags(); if (tags != null) { - for(Tag tag : tags){ + for(String tag : tags){ cmd.add("t/" + tag); } }