-
Notifications
You must be signed in to change notification settings - Fork 270
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
[Edward Alvin] ip #305
base: master
Are you sure you want to change the base?
[Edward Alvin] ip #305
Conversation
[Edward Alvin] Create Duke Skeleton, ip
Add new Exceptions for basic invalid inputs
src/main/java/Duke.java
Outdated
return Duke.LIST.equals(word); | ||
public static void processInput(InputType inputType, String value) { | ||
switch(inputType) { | ||
case LIST: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the codestyle, there should be no intentation for 'case' clauses in the switch block.
src/main/java/Duke.java
Outdated
public static boolean isList(String word) { | ||
return Duke.LIST.equals(word); | ||
public static void processInput(InputType inputType, String value) { | ||
switch(inputType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style-related thing, but there should be a space before the bracket i.e switch (inputType)
src/main/java/Duke.java
Outdated
@@ -25,7 +25,7 @@ public static void main(String[] args) { | |||
|
|||
Object[] parseResult = InputParser.parseInput(input); | |||
InputType inputType = (InputType) parseResult[0]; | |||
String value = (String) parseResult[1]; | |||
String[] value = (String[]) parseResult[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the cast to String[] array here, instead of making InputParser::parseInput(input)
return a String[] instead of Object[]?
src/main/java/InputParser.java
Outdated
value = ""; | ||
if (inputType.label.length() < input.length()) { | ||
value = input.substring(inputType.label.length() + 1); | ||
if (inputType == InputType.BYE || inputType == InputType.LIST) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be neater to separate out into boolean variables for example isBye
, isList
, etc to be in line with the code quality guideline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I found it quite easy to read all the code. I think you just need to adjust some small things like renaming arrays and arraylists to be plural form.
src/main/java/Duke.java
Outdated
Object[] parseResult = InputParser.parseInput(input); | ||
InputType inputType = (InputType) parseResult[0]; | ||
String[] value = (String[]) parseResult[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think parseResult and value could both be renamed in plural form (results and values perhaps?) to be consistent with naming conventions. I noticed this in a few other places.
src/main/java/Duke.java
Outdated
} | ||
} | ||
|
||
public static void processInput(InputType inputType, String[] value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small thing because its just a parameter but maybe change value to values so its plural?
src/main/java/Todo.java
Outdated
@@ -0,0 +1,12 @@ | |||
public class Todo extends WordListItem{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be a space between WordListItem and {.
@@ -0,0 +1,5 @@ | |||
public class UnknownInputException extends Exception{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be a space between Exception and {.
src/main/java/WordList.java
Outdated
import java.util.ArrayList; | ||
|
||
public class WordList { | ||
private ArrayList<WordListItem> wordList; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could be renamed to words or something similar to be plural?
src/main/java/WordList.java
Outdated
System.out.println(this); | ||
} | ||
|
||
public int length() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The coding standards suggest that method names be verbs. Perhaps this could be changed to getLength or something like that.
…efactor the rest of the code
The sort command does not exist yet. The sort command allows more functionality for the user which will come really handy. Let's, * create the sort, sortevent, and sortdeadline command in inputtype * update the inputparser and wordlist to adjust
Duked
Duked frees your mind of having to remember things you need to do. It's,
All you need to do is,
And it is FREE! Features:
Just double click on the jar and it should open the app GUI :) or Run java -jar Duke.jar //depending on the relative location of the app |
No description provided.