Fix storage bug

docs/DeveloperGuide.adoc

@@ -509,28 +509,20 @@ Additionally, doing these actions when parsing, though bloats code, allows for m
 The find function supports searching using a single keyword. +
 If some part of the name of `Slot` / tags attributed to `Slot` contains the specified keyword,
 a positive match is registered and the designated `Slot` will be added to the output list. +
-Examples: +
-`find n/CS` +
-`CS2101`, `CS2113T`, `SOCSMeet` are considered positive matches. +
-`ST2334`, `Hockey`, `Lunch` are NOT considered positive matches. +
 
 ==== Design Considerations
 
 ===== Aspect: What constitutes a positive search result in `find` command
 * **Alternative 1:** Positive search result by strictly matching the entered keyword
 ** Pros: Easy to implement.
 ** Cons: Search must be exact, typos or an incomplete keyword will yield incorrect results. Nothing different from `List` feature.
-* **Alternative 2 (current choice):** Positive search result as long as name/tag contains the keyword.
-** Pros: Searches can now consider names/tags similar to the keyword.
+* **Alternative 2:** Positive search result as long as name/tag contains the keyword.
+** Pros: Searches will detect names/tags similar to the keyword.
 ** Cons: Output list will be longer. May become excessively long if short keyword is provided.
-* **Alternative 3 (future implementation):** Store the search results in a `treeMap` ordered by their Levenshtein distances from the search keyword
-** Pros: Will also consider searches that are similar to what we want and will account for typos or incomplete keywords
+* **Alternative 3 (current implementation):** Store the search results in a `PriorityQueue` ordered by their <<levenshtein-distance,Levenshtein distances>> from the search keyword.
+** Pros: Will also consider searches that are similar to the desired name/tag and will account for a typo or an incomplete keyword
 ** Cons: Added complexities in finding and searching.
 
-==== Future Implementation
-The find function will be revamped to support search guessing. +
-FindCommand will be enhanced by using Levenshtein Distance to generate an ordered set of `Slot` attributes ordered by similarity.
-
 === View Month/Week/Day feature
 
 ==== Design Considerations
@@ -1379,6 +1371,11 @@ _{More to be added}_
 [[mainstream-os]] Mainstream OS::
 Windows, Linux, Unix, OS-X
 
+[[levenshtein-distance]] Levenshtein Distance::
+The Levenshtein distance is a string metric for measuring difference between two sequences. +
+Informally, the Levenshtein distance between two words is the minimum number of single-character edits (i.e. insertions, deletions or substitutions)
+required to change one word into the other.
+
 ////
 Should only include this if we actually make any mention to private planner detail.
 [[private-planner-detail]] Private planner detail::

src/planmysem/Main.java

@@ -4,8 +4,6 @@
 import javafx.application.Platform;
 import javafx.stage.Stage;
 import planmysem.logic.LogicManager;
-import planmysem.model.Model;
-import planmysem.model.ModelManager;
 import planmysem.storage.Storage;
 import planmysem.storage.StorageFile;
 import planmysem.ui.Gui;
@@ -28,8 +26,7 @@ public static void main(String[] args) {
     @Override
     public void start(Stage primaryStage) throws Exception {
         Storage storageFile = new StorageFile();
-        Model modelManager = new ModelManager();
-        Gui gui = new Gui(new LogicManager(storageFile, modelManager), VERSION);
+        Gui gui = new Gui(new LogicManager(storageFile), VERSION);
         gui.start(primaryStage, this);
     }
 

src/planmysem/logic/LogicManager.java

@@ -11,6 +11,7 @@
 import planmysem.logic.parser.ParserManager;
 import planmysem.logic.parser.exceptions.ParseException;
 import planmysem.model.Model;
+import planmysem.model.ModelManager;
 import planmysem.model.semester.ReadOnlyDay;
 import planmysem.model.slot.ReadOnlySlot;
 import planmysem.storage.Storage;
@@ -27,9 +28,9 @@ public class LogicManager implements Logic {
     private final CommandHistory history;
     private final ParserManager parserManager;
 
-    public LogicManager(Storage storageFile, Model model) {
+    public LogicManager(Storage storageFile) throws Exception {
         this.storageFile = storageFile;
-        this.model = model;
+        this.model = new ModelManager(storageFile.load());
         this.history = new CommandHistory();
         this.parserManager = new ParserManager();
     }
@@ -54,7 +55,7 @@ public CommandResult execute(String userCommandText) throws CommandException, Pa
             history.add(userCommandText);
         }
         try {
-            storageFile.save(model);
+            storageFile.save(model.getPlanner());
         } catch (StorageFile.StorageOperationException soe) {
             throw new CommandException(STORAGE_ERROR + soe, soe);
         }

src/planmysem/logic/commands/AddCommand.java

@@ -70,15 +70,14 @@ public AddCommand(Slot slot, Recurrence recurrence) {
     public CommandResult execute(Model model, CommandHistory commandHistory) throws CommandException {
         Set<LocalDate> dates = recurrence.generateDates(model.getPlanner().getSemester());
         Map<LocalDate, Day> days = new TreeMap<>();
-
         for (LocalDate date : dates) {
             try {
                 days.put(date, model.addSlot(date, slot));
             } catch (Semester.DateNotFoundException dnfe) {
                 throw new CommandException(MESSAGE_FAIL_OUT_OF_BOUNDS);
             }
         }
-
+        model.commit();
         return new CommandResult(String.format(MESSAGE_SUCCESS, dates.size(),
                 craftSuccessMessage(days, slot)));
     }

src/planmysem/logic/commands/DeleteCommand.java

@@ -85,7 +85,7 @@ public CommandResult execute(Model model, CommandHistory commandHistory) throws
                 throw new CommandException(Messages.MESSAGE_INVALID_SLOT_DISPLAYED_INDEX);
             }
         }
-
+        model.commit();
         return new CommandResult(String.format(MESSAGE_SUCCESS,
                 selectedSlots.size(), messageSelected, messageSlots));
     }

src/planmysem/logic/commands/EditCommand.java

@@ -126,7 +126,7 @@ public CommandResult execute(Model model, CommandHistory commandHistory) throws
                 throw new CommandException(Messages.MESSAGE_INVALID_SLOT_DISPLAYED_INDEX);
             }
         }
-
+        model.commit();
         return new CommandResult(String.format(MESSAGE_SUCCESS, selectedSlots.size(),
                 messageSelected, messageSlots));
     }

src/planmysem/logic/commands/RedoCommand.java

@@ -1,3 +1,4 @@
+//@@author marcus-pzj
 package planmysem.logic.commands;
 
 import static java.util.Objects.requireNonNull;
@@ -7,7 +8,7 @@
 import planmysem.model.Model;
 
 /**
- * Reverts the {@code model}'s address book to its previously undone state.
+ * Reverts the {@code model}'s planner to its previously undone state.
  */
 public class RedoCommand extends Command {
 

src/planmysem/logic/commands/UndoCommand.java

@@ -1,3 +1,4 @@
+//@@author marcus-pzj
 package planmysem.logic.commands;
 
 import static java.util.Objects.requireNonNull;
@@ -7,7 +8,7 @@
 import planmysem.model.Model;
 
 /**
- * Reverts the {@code model}'s address book to its previous state.
+ * Reverts the {@code model}'s planner to its previous state.
  */
 public class UndoCommand extends Command {
 

src/planmysem/model/VersionedPlanner.java

@@ -4,7 +4,7 @@
 import java.util.List;
 
 /**
- * {@code AddressBook} that keeps track of its own history.
+ * {@code Planner} that keeps track of its own history.
  */
 public class VersionedPlanner extends Planner {
 
@@ -20,7 +20,7 @@ public VersionedPlanner(ReadOnlyPlanner initialState) {
     }
 
     /**
-     * Saves a copy of the current {@code AddressBook} state at the end of the state list.
+     * Saves a copy of the current {@code Planner} state at the end of the state list.
      * Undone states are removed from the state list.
      */
     public void commit() {
@@ -34,7 +34,7 @@ private void removeStatesAfterCurrentPointer() {
     }
 
     /**
-     * Restores the address book to its previous state.
+     * Restores the planner to its previous state.
      */
     public void undo() {
         if (!canUndo()) {
@@ -45,7 +45,7 @@ public void undo() {
     }
 
     /**
-     * Restores the address book to its previously undone state.
+     * Restores the planner to its previously undone state.
      */
     public void redo() {
         if (!canRedo()) {
@@ -56,14 +56,14 @@ public void redo() {
     }
 
     /**
-     * Returns true if {@code undo()} has address book states to undo.
+     * Returns true if {@code undo()} has planner states to undo.
      */
     public boolean canUndo() {
         return currentStatePointer > 0;
     }
 
     /**
-     * Returns true if {@code redo()} has address book states to redo.
+     * Returns true if {@code redo()} has planner states to redo.
      */
     public boolean canRedo() {
         return currentStatePointer < plannerListState.size() - 1;
@@ -94,7 +94,7 @@ public boolean equals(Object other) {
      */
     public static class NoUndoableStateException extends RuntimeException {
         private NoUndoableStateException() {
-            super("Current state pointer at start of addressBookState list, unable to undo.");
+            super("Current state pointer at start of plannerState list, unable to undo.");
         }
     }
 
@@ -103,7 +103,7 @@ private NoUndoableStateException() {
      */
     public static class NoRedoableStateException extends RuntimeException {
         private NoRedoableStateException() {
-            super("Current state pointer at end of addressBookState list, unable to redo.");
+            super("Current state pointer at end of plannerState list, unable to redo.");
         }
     }
 }

src/planmysem/model/semester/Day.java

@@ -24,7 +24,7 @@ public Day(ReadOnlyDay day) {
         this.dayOfWeek = day.getDayOfWeek();
         this.type = day.getType();
         for (Slot slot : day.getSlots()) {
-            this.slots.add(slot);
+            this.slots.add(new Slot(slot));
         }
     }
 
@@ -38,15 +38,15 @@ public Day(DayOfWeek dayOfWeek, String weekType, ArrayList<Slot> slots) {
         this.type = weekType;
 
         for (Slot slot : slots) {
-            this.slots.add(slot);
+            this.slots.add(new Slot(slot));
         }
     }
 
     /**
      * Add a slot to the day.
      */
     public void addSlot(Slot slot) {
-        slots.add(slot);
+        slots.add(new Slot(slot));
     }
 
     /**

src/planmysem/model/semester/Semester.java

@@ -467,7 +467,10 @@ public String getAcademicYear() {
     @Override
     public void setDays(HashMap<LocalDate, Day> days) {
         this.days.clear();
-        this.days.putAll(days);
+
+        for (Map.Entry<LocalDate, Day> entry : days.entrySet()) {
+            this.days.put(entry.getKey(), new Day(entry.getValue()));
+        }
     }
 
     @Override

src/planmysem/storage/Storage.java

@@ -1,7 +1,7 @@
 package planmysem.storage;
 
 import planmysem.common.exceptions.IllegalValueException;
-import planmysem.model.Model;
+import planmysem.model.Planner;
 
 /**
  * API of the Logic component
@@ -13,14 +13,14 @@ public interface Storage {
      *
      * @throws StorageFile.StorageOperationException if there were errors converting and/or storing model to file.
      */
-    void save(Model model) throws StorageFile.StorageOperationException;
+    void save(Planner planner) throws StorageFile.StorageOperationException;
 
     /**
      * Loads model from this storage file.
      *
      * @throws StorageFile.StorageOperationException if there were errors reading and/or converting model from file.
      */
-    Model load() throws StorageFile.StorageOperationException;
+    Planner load() throws StorageFile.StorageOperationException;
 
     /**
      * Gets path of file.

src/planmysem/storage/StorageFile.java

@@ -18,8 +18,7 @@
 import javax.xml.bind.Unmarshaller;
 
 import planmysem.common.exceptions.IllegalValueException;
-import planmysem.model.Model;
-import planmysem.model.ModelManager;
+import planmysem.model.Planner;
 import planmysem.storage.jaxb.AdaptedPlanner;
 
 /**
@@ -70,14 +69,14 @@ private static boolean isValidPath(Path filePath) {
     }
 
     @Override
-    public void save(Model model) throws StorageOperationException {
+    public void save(Planner planner) throws StorageOperationException {
         /* Note: Note the 'try with resource' statement below.
          * More info: https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
          */
         try (final Writer fileWriter =
                      new BufferedWriter(new FileWriter(path.toFile()))) {
 
-            final AdaptedPlanner toSave = new AdaptedPlanner(model.getPlanner());
+            final AdaptedPlanner toSave = new AdaptedPlanner(planner);
             final Marshaller marshaller = jaxbContext.createMarshaller();
             marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
             if (isEncrypted) {
@@ -96,7 +95,7 @@ public void save(Model model) throws StorageOperationException {
     }
 
     @Override
-    public Model load() throws StorageOperationException {
+    public Planner load() throws StorageOperationException {
         try (final BufferedReader fileReader =
                      new BufferedReader(new FileReader(path.toFile()))) {
 
@@ -114,7 +113,7 @@ public Model load() throws StorageOperationException {
             if (loaded.isAnyRequiredFieldMissing()) {
                 throw new StorageOperationException("File model missing some elements");
             }
-            return new ModelManager(loaded.toModelType());
+            return loaded.toModelType();
 
             /* Note: Here, we are using an exception to create the file if it is missing or empty. However, we should
              * minimize using exceptions to facilitate normal paths of execution. If we consider the missing file as a
@@ -123,7 +122,7 @@ public Model load() throws StorageOperationException {
 
             // create empty file if not found or is empty
         } catch (FileNotFoundException | NullPointerException ex) {
-            final Model empty = new ModelManager();
+            final Planner empty = new Planner();
             save(empty);
             return empty;
 

src/planmysem/storage/jaxb/AdaptedSlot.java

@@ -75,7 +75,8 @@ public boolean isAnyRequiredFieldMissing() {
      * @throws IllegalValueException if there were any model constraints violated in the IcsSemester
      */
     public Slot toModelType() throws IllegalValueException {
-        if (hasIllegalValues(name) || hasIllegalValues(location) || hasIllegalValues(description) || duration < 0) {
+        if ((name != null && hasIllegalValues(name)) || (location != null && hasIllegalValues(location))
+                || (description != null && hasIllegalValues(description)) || duration < 0) {
             throw new IllegalValueException(MESSAGE_ILLEGAL_VALUE);
         }
 

test/java/planmysem/logic/Commands/AddCommandTest.java

@@ -135,7 +135,6 @@ public void clearLastShownList() {
 
         @Override
         public void commit() {
-            throw new AssertionError("This method should not be called.");
         }
 
         @Override

test/java/planmysem/logic/Commands/DeleteCommandTest.java

@@ -131,6 +131,7 @@ public void execute_validTag_success() {
 
         expectedModel.removeSlot(pair3);
         expectedModel.removeSlot(pair4);
+        expectedModel.commit();
 
         assertCommandSuccess(deleteCommand, model, commandHistory, expectedMessage, expectedModel);
     }
@@ -147,6 +148,7 @@ public void execute_validIndex_success() {
                 Messages.craftSelectedMessage("Deleted Slot:", selectedSlots));
 
         expectedModel.removeSlot(slot);
+        expectedModel.commit();
 
         assertCommandSuccess(deleteCommand, model, commandHistory, expectedMessage, expectedModel);
     }

test/java/planmysem/logic/Commands/EditCommandTest.java

@@ -167,7 +167,7 @@ public void execute_validTag_success() {
                 description,
                 tags
         );
-
+        expectedModel.commit();
         assertCommandSuccess(editCommand, model, commandHistory, expectedMessage, expectedModel);
     }
 
@@ -269,6 +269,7 @@ public void execute_validIndex_success() {
                 description,
                 tags
         );
+        expectedModel.commit();
 
         assertCommandSuccess(editCommand, model, commandHistory, expectedMessage, expectedModel);
     }