From fbf998425970c54a6aa83d9412ec9d5f712647bc Mon Sep 17 00:00:00 2001 From: Vishnu Prem Date: Wed, 2 Nov 2016 08:37:56 +0800 Subject: [PATCH] Addressing tutor comments (#189) * Abstract some code in Parser * Add comments --- .../agendum/logic/parser/DateTimeUtils.java | 15 +++++ .../logic/parser/EditDistanceCalculator.java | 39 ++++++++--- .../seedu/agendum/logic/parser/Parser.java | 67 +++++++++++-------- .../java/seedu/agendum/ui/CommandBox.java | 2 +- .../java/seedu/agendum/ui/HelpWindow.java | 8 ++- .../logic/EditDistanceCalculatorTest.java | 20 +++--- 6 files changed, 101 insertions(+), 50 deletions(-) diff --git a/src/main/java/seedu/agendum/logic/parser/DateTimeUtils.java b/src/main/java/seedu/agendum/logic/parser/DateTimeUtils.java index 1bb4b43937f0..3e7e9fa69e7f 100644 --- a/src/main/java/seedu/agendum/logic/parser/DateTimeUtils.java +++ b/src/main/java/seedu/agendum/logic/parser/DateTimeUtils.java @@ -10,8 +10,16 @@ //@@author A0003878Y +/** + * Utilities for DateTime parsing + */ public class DateTimeUtils { + /** + * Parses input string into LocalDateTime objects using Natural Language Parsing + * @param input natural language date time string + * @return Optional is null if input coult not be parsed + */ public static Optional parseNaturalLanguageDateTimeString(String input) { if(input == null || input.isEmpty()) { return Optional.empty(); @@ -36,6 +44,13 @@ public static Optional parseNaturalLanguageDateTimeString(String return Optional.ofNullable(localDateTime); } + /** + * Takes two LocalDateTime and balances by ensuring that the latter DateTime is gaurenteed to be later + * than the former DateTime + * @param startDateTime + * @param endDateTime + * @return endDateTime that is now balanced + */ public static LocalDateTime balanceStartAndEndDateTime(LocalDateTime startDateTime, LocalDateTime endDateTime) { LocalDateTime newEndDateTime = endDateTime; while (startDateTime.compareTo(newEndDateTime) >= 1) { diff --git a/src/main/java/seedu/agendum/logic/parser/EditDistanceCalculator.java b/src/main/java/seedu/agendum/logic/parser/EditDistanceCalculator.java index 461f46960753..c60c36755284 100644 --- a/src/main/java/seedu/agendum/logic/parser/EditDistanceCalculator.java +++ b/src/main/java/seedu/agendum/logic/parser/EditDistanceCalculator.java @@ -12,15 +12,21 @@ import java.util.logging.Logger; //@@author A0003878Y + +/** + * A static class for calculating levenshtein distance between two strings + */ public class EditDistanceCalculator { private static final Logger logger = LogsCenter.getLogger(EditDistanceCalculator.class); private static final int EDIT_DISTANCE_THRESHOLD = 3; + /** + * Attempts to find the 'closest' command for an input String + * @param input user inputted command + * @return Optional string that's the closest command to input. Null if not found. + */ public static Optional closestCommandMatch(String input) { - Reflections reflections = new Reflections("seedu.agendum"); - Set> classes = reflections.getSubTypesOf(Command.class); - final String[] bestCommand = {""}; final int[] bestCommandDistance = {Integer.MAX_VALUE}; @@ -32,7 +38,6 @@ public static Optional closestCommandMatch(String input) { bestCommandDistance[0] = commandWordDistance; } }; - executeOnAllCommands(consumer); if (bestCommandDistance[0] < EDIT_DISTANCE_THRESHOLD) { @@ -42,7 +47,13 @@ public static Optional closestCommandMatch(String input) { } } - public static Optional commandCompletion(String input) { + /** + * Attempts to 'complete' the input String into an actual command + * @param input user inputted command + * @return Optional string that's command that best completes the input. If input matches more than + * one command, null ire returned. Null is also returned if a command is not found. + */ + public static Optional findCommandCompletion(String input) { ArrayList matchedCommands = new ArrayList<>(); Consumer consumer = (commandWord) -> { @@ -50,7 +61,6 @@ public static Optional commandCompletion(String input) { matchedCommands.add(commandWord); } }; - executeOnAllCommands(consumer); if (matchedCommands.size() == 1) { @@ -60,14 +70,19 @@ public static Optional commandCompletion(String input) { } } - private static void executeOnAllCommands(Consumer f) { + /** + * A higher order method that takes in an operation to perform on all Commands using + * Java reflection and functional programming paradigm. + * @param f A closure that takes a String as input that executes on all Commands. + */ + private static void executeOnAllCommands(Consumer f) { new Reflections("seedu.agendum").getSubTypesOf(Command.class) .stream() .map(s -> { try { return s.getMethod("getName").invoke(null).toString(); } catch (NullPointerException e) { - return ""; + return ""; // Suppress this exception are we expect some Commands to not conform to getName() } catch (Exception e) { logger.severe("Java reflection for Command class failed"); throw new RuntimeException(); @@ -78,7 +93,13 @@ private static void executeOnAllCommands(Consumer f) { } - // Code from https://rosettacode.org/wiki/Levenshtein_distance#Java + /** + * Calculates levenshtein distnace between two strings. + * Code from https://rosettacode.org/wiki/Levenshtein_distance#Java + * @param a + * @param b + * @return + */ private static int distance(String a, String b) { a = a.toLowerCase(); b = b.toLowerCase(); diff --git a/src/main/java/seedu/agendum/logic/parser/Parser.java b/src/main/java/seedu/agendum/logic/parser/Parser.java index 5a856c7d7d7b..e1941b80dbc5 100644 --- a/src/main/java/seedu/agendum/logic/parser/Parser.java +++ b/src/main/java/seedu/agendum/logic/parser/Parser.java @@ -6,6 +6,7 @@ import java.time.LocalDateTime; import java.util.*; +import java.util.function.BiConsumer; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -150,30 +151,28 @@ private Command prepareAdd(String args) { try { matcher.reset(); matcher.find(); - String taskTitle = matcher.group(0); + StringBuilder titleBuilder = new StringBuilder(matcher.group(0)); HashMap> dateTimeMap = new HashMap<>(); - while (matcher.find()) { - for (String token : TIME_TOKENS) { - String s = matcher.group(0).toLowerCase(); - if (s.startsWith(token)) { - String time = s.substring(token.length(), s.length()); - if (DateTimeUtils.containsTime(time)) { - dateTimeMap.put(token, DateTimeUtils.parseNaturalLanguageDateTimeString(time)); - } else { - taskTitle = taskTitle + s; - } - } + BiConsumer consumer = (matchedGroup, token) -> { + String time = matchedGroup.substring(token.length(), matchedGroup.length()); + if (DateTimeUtils.containsTime(time)) { + dateTimeMap.put(token, DateTimeUtils.parseNaturalLanguageDateTimeString(time)); + } else { + titleBuilder.append(matchedGroup); } - } + }; + scheduleMatcherOnConsumer(matcher, consumer); + + String title = titleBuilder.toString(); if (dateTimeMap.containsKey(ARGS_BY)) { - return new AddCommand(taskTitle, dateTimeMap.get(ARGS_BY)); + return new AddCommand(title, dateTimeMap.get(ARGS_BY)); } else if (dateTimeMap.containsKey(ARGS_FROM) && dateTimeMap.containsKey(ARGS_TO)) { - return new AddCommand(taskTitle, dateTimeMap.get(ARGS_FROM), dateTimeMap.get(ARGS_TO)); + return new AddCommand(title, dateTimeMap.get(ARGS_FROM), dateTimeMap.get(ARGS_TO)); } else if (!dateTimeMap.containsKey(ARGS_FROM) && !dateTimeMap.containsKey(ARGS_TO) && !dateTimeMap.containsKey(ARGS_BY)) { - return new AddCommand(taskTitle); + return new AddCommand(title); } else { return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE)); @@ -199,6 +198,7 @@ private Command prepareSchedule(String args) { matcher.reset(); matcher.find(); + HashMap> dateTimeMap = new HashMap<>(); Optional taskIndex = parseIndex(matcher.group(0)); int index = 0; if (taskIndex.isPresent()) { @@ -207,20 +207,14 @@ private Command prepareSchedule(String args) { return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, ScheduleCommand.MESSAGE_USAGE)); } - - HashMap> dateTimeMap = new HashMap<>(); - while (matcher.find()) { - for (String token : TIME_TOKENS) { - String s = matcher.group(0).toLowerCase(); - if (s.startsWith(token)) { - String time = s.substring(token.length(), s.length()); - if (DateTimeUtils.containsTime(time)) { - dateTimeMap.put(token, DateTimeUtils.parseNaturalLanguageDateTimeString(time)); - } - } + BiConsumer consumer = (matchedGroup, token) -> { + String time = matchedGroup.substring(token.length(), matchedGroup.length()); + if (DateTimeUtils.containsTime(time)) { + dateTimeMap.put(token, DateTimeUtils.parseNaturalLanguageDateTimeString(time)); } - } + }; + scheduleMatcherOnConsumer(matcher, consumer); if (dateTimeMap.containsKey(ARGS_BY)) { return new ScheduleCommand(index, Optional.empty(), dateTimeMap.get(ARGS_BY)); @@ -234,6 +228,23 @@ private Command prepareSchedule(String args) { String.format(MESSAGE_INVALID_COMMAND_FORMAT, ScheduleCommand.MESSAGE_USAGE)); } } + + /** + * Parses arguments in the context of the schedule task command. + * + * @param matcher matcher for current command context + * @param consumer closure to execute on + */ + private void scheduleMatcherOnConsumer(Matcher matcher, BiConsumer consumer) { + while (matcher.find()) { + for (String token : TIME_TOKENS) { + String matchedGroup = matcher.group(0).toLowerCase(); + if (matchedGroup.startsWith(token)) { + consumer.accept(matchedGroup, token); + } + } + } + } //@@author A0133367E diff --git a/src/main/java/seedu/agendum/ui/CommandBox.java b/src/main/java/seedu/agendum/ui/CommandBox.java index 979dd62b8a92..bb19d0a02f31 100644 --- a/src/main/java/seedu/agendum/ui/CommandBox.java +++ b/src/main/java/seedu/agendum/ui/CommandBox.java @@ -145,7 +145,7 @@ private void registerTabKeyEventFilter() { commandTextField.addEventFilter(KeyEvent.KEY_PRESSED, event -> { KeyCode keyCode = event.getCode(); if (keyCode.equals(KeyCode.TAB)) { - Optional parsedString = EditDistanceCalculator.commandCompletion(commandTextField.getText()); + Optional parsedString = EditDistanceCalculator.findCommandCompletion(commandTextField.getText()); if(parsedString.isPresent()) { commandTextField.setText(parsedString.get()); } diff --git a/src/main/java/seedu/agendum/ui/HelpWindow.java b/src/main/java/seedu/agendum/ui/HelpWindow.java index f16f5d1d321e..0104386821ec 100644 --- a/src/main/java/seedu/agendum/ui/HelpWindow.java +++ b/src/main/java/seedu/agendum/ui/HelpWindow.java @@ -108,8 +108,12 @@ public void show(double height) { } //@@author A0003878Y + + /** + * Uses Java reflection followed by Java stream.map() to retrieve all commands for listing on the Help + * window dynamically + */ private void loadHelpList() { - new Reflections("seedu.agendum").getSubTypesOf(Command.class) .stream() .map(s -> { @@ -120,7 +124,7 @@ private void loadHelpList() { map.put(CommandColumns.DESCRIPTION, s.getMethod("getDescription").invoke(null).toString()); return map; } catch (NullPointerException e) { - return null; + return null; // Suppress this exception are we expect some Commands to not conform to these methods } catch (Exception e) { logger.severe("Java reflection for Command class failed"); throw new RuntimeException(); diff --git a/src/test/java/seedu/agendum/logic/EditDistanceCalculatorTest.java b/src/test/java/seedu/agendum/logic/EditDistanceCalculatorTest.java index b55322ab0d49..ca977e1c38df 100644 --- a/src/test/java/seedu/agendum/logic/EditDistanceCalculatorTest.java +++ b/src/test/java/seedu/agendum/logic/EditDistanceCalculatorTest.java @@ -24,16 +24,16 @@ public void closestCommandMatchTest() throws Exception { @Test public void commandCompletion() throws Exception { - assertEquals(EditDistanceCalculator.commandCompletion("ad").get(), "add"); - assertEquals(EditDistanceCalculator.commandCompletion("ma").get(), "mark"); - assertEquals(EditDistanceCalculator.commandCompletion("un"), Optional.empty()); // ambiguous returns nothing. Can be undo or unmark - assertEquals(EditDistanceCalculator.commandCompletion("unm").get(), "unmark"); - assertEquals(EditDistanceCalculator.commandCompletion("und").get(), "undo"); - assertEquals(EditDistanceCalculator.commandCompletion("st").get(), "store"); - assertEquals(EditDistanceCalculator.commandCompletion("de").get(), "delete"); - assertEquals(EditDistanceCalculator.commandCompletion("he").get(), "help"); - assertEquals(EditDistanceCalculator.commandCompletion("sc").get(), "schedule"); - assertEquals(EditDistanceCalculator.commandCompletion("r").get(), "rename"); + assertEquals(EditDistanceCalculator.findCommandCompletion("ad").get(), "add"); + assertEquals(EditDistanceCalculator.findCommandCompletion("ma").get(), "mark"); + assertEquals(EditDistanceCalculator.findCommandCompletion("un"), Optional.empty()); // ambiguous returns nothing. Can be undo or unmark + assertEquals(EditDistanceCalculator.findCommandCompletion("unm").get(), "unmark"); + assertEquals(EditDistanceCalculator.findCommandCompletion("und").get(), "undo"); + assertEquals(EditDistanceCalculator.findCommandCompletion("st").get(), "store"); + assertEquals(EditDistanceCalculator.findCommandCompletion("de").get(), "delete"); + assertEquals(EditDistanceCalculator.findCommandCompletion("he").get(), "help"); + assertEquals(EditDistanceCalculator.findCommandCompletion("sc").get(), "schedule"); + assertEquals(EditDistanceCalculator.findCommandCompletion("r").get(), "rename"); } }