-
Notifications
You must be signed in to change notification settings - Fork 454
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
[Shui Jie] ip #493
base: master
Are you sure you want to change the base?
[Shui Jie] ip #493
Conversation
Used an ArrayList to store list of items
Simplified overall structure. Added an ArrayList as global variable. changed the property of done.
Added documentation to all the functions. Changed the choiceOfAction function such that it can recognize different task. setDate function to create the date String
Changed the structure of the program such that the three task types are all subtask of the main Task
Created a test inputs and outputs
handle errors involving format problems for all three tasks. Exceptions is utilized.
Implement a deleteTask function to remove task when needed
This reverts commit af273dc.
Added 3 functions to save list into file, and then read them upon reopening.
Added in LocalDate and DateTimeFormatter. Changed Deadline and Events Task restrictors as the format has now changed.
src/main/java/Duke.java
Outdated
public class Duke { | ||
|
||
private String line = "__________________________________\n"; |
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 feel this should be a final field?
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.
It seems that line is a constant. Maybe calling it LINE is more suitable for the coding standard.
src/main/java/Duke.java
Outdated
* get isDone | ||
* @return | ||
*/ | ||
public boolean getIsDone() { |
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 should just be isDone()?
src/main/java/Duke.java
Outdated
return output + " )"; | ||
} | ||
/** | ||
* modify the task message if done. |
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 you should try to capitalise all JavaDocs descriptions.
src/main/java/Duke.java
Outdated
* @param input | ||
* @return boolean | ||
*/ | ||
private boolean choiceOfAction(String input) { |
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.
Since this is a function that returns a boolean result, I do feel the naming should be isSetDone or isNewTask.
src/main/java/Duke.java
Outdated
writer.write(formatTaskIntoCommands(task)); | ||
} | ||
writer.close(); | ||
}catch (IOException io) { |
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.
Spacing error.
src/main/java/Duke.java
Outdated
try { | ||
Task newTask = createTask(input); | ||
addTask(newTask); | ||
} catch (TodoException tx) { |
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 feel that seperating exception for each class is quite unneccesary. Simply a TaskException would be easier to understand.
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.
Overall not too many coding standard violations.
The code would look neater in packages.
src/main/java/Duke.java
Outdated
public class Duke { | ||
|
||
private String line = "__________________________________\n"; |
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.
It seems that line is a constant. Maybe calling it LINE is more suitable for the coding standard.
src/main/java/Duke.java
Outdated
*/ | ||
private void greet() { | ||
System.out.println(line); | ||
String logo = " ____ _ \n" |
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.
Would it be better for logo to be declared as a constant?
src/main/java/Duke.java
Outdated
} | ||
} | ||
}catch (FileNotFoundException f) { | ||
}catch(IOException io){ |
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.
There should be a space before the open curly bracket.
src/main/java/Duke.java
Outdated
*/ | ||
public TodoTasks(String action) throws TodoException{ | ||
super(action, "[T][] ", TaskTypes.Todo); | ||
if (action.split(" ").length < 2){ |
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.
There should be a space before the curly bracket.
Restructured the entire file to a more OOP structure. Java Doc is also updated.
Create 2 packages, Task and Storage. Subdivide classes into respective public class file.
forgot to add Duke package
Created two Junit Test to test the saving of file, and whether invalid inputs is not saved.
Created a workable Duke.java.jar file
Give users a way to find a task by searching for a keyword. Restructured ChoiceOfAction. Now it only chooses which Action Function to use. New function doneAction, deleteAction and findAction.
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.
LGTM! Just some nit that is related to coding stnadards to fix
src/main/java/Duke/DukeTest.java
Outdated
|
||
public class DukeTest { | ||
@Test | ||
public void dummyTest(){ |
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.
public void dummyTest(){ | |
public void dummyTest() { |
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 can add a space?
@@ -0,0 +1,203 @@ | |||
package Storage; | |||
|
|||
import java.io.*; |
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 can list out the things you want to import instead of using a wildcard?
import Task.EventsException; | ||
|
||
public class Storage { | ||
private final Parser parser= new Parser(); |
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.
private final Parser parser= new Parser(); | |
private final Parser PARSER = new Parser(); |
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 if you are declaring a constant, it needs to be in caps. Also, don't forget the space before the "=".
|
||
// deals with making sense of the user command | ||
class Parser { | ||
|
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.
Don't forget to remove this empty line
src/main/java/Duke/DukeTest.java
Outdated
|
||
@Test | ||
public void testInvalidFormat() { | ||
|
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.
Don't forget to remove this empty line too
Changed many functions from TaskList and UI class such that they can return the message as String instead of printing it in the console. Setup the GUI which can print the results.
mistakes were made, this is the actual level 10
This reverts commit 8e28087.
1.Created a fat jar file underout\artifact\ip.jar.
added new function isTaskDuplicate under parser. The program can check if the input value will create a program that is already present in the tasklist
added new function isTaskDuplicate under parser. The program can check if the input value will create a program that is already present in the tasklist
Improved code quality 1. changed multiple if-elseif statement to switch 2. Added new function comments Remove a bug that causes program to print logo instead of parsing the input
Improved code quality 1. changed multiple if-elseif statement to switch 2. Added new function comments Remove a bug that causes program to print logo instead of parsing the input
Improve code quality
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.
Overall, the work is excellent.
Some methods could be shortened perhaps.
Beyond that, great job!
public void getDataInputList(TaskList lst) { | ||
try { | ||
File f = new File("src/main/java/Duke.txt"); | ||
if(!f.exists()){ | ||
f.createNewFile(); | ||
} | ||
FileReader reader = new FileReader(f); | ||
BufferedReader br = new BufferedReader(reader); //creates a buffering character input stream | ||
String line; | ||
Task curTask; | ||
|
||
while ((line = br.readLine()) != null) { | ||
boolean isCurTaskDone = false; | ||
String[] data = line.split(" "); | ||
int lengthLimit = data.length; | ||
String taskData = ""; | ||
|
||
if (data[data.length - 1].equals("done")){ | ||
isCurTaskDone = true; | ||
lengthLimit -= 1; | ||
} | ||
try { | ||
for (int i = 0; i < lengthLimit; i++) { | ||
taskData = taskData.concat(data[i] + " "); | ||
} | ||
curTask = parser.createTask(taskData); | ||
if (isCurTaskDone) { | ||
curTask.done(); | ||
} | ||
lst.getTaskList().add(curTask); | ||
} catch (Exception e) { | ||
System.out.println(e.getLocalizedMessage()); | ||
} | ||
} |
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 method exceeds 40 lines.
Method could be SLAP-ed harder.
class TodoTasks extends Task{ | ||
/** | ||
* constructor for Task. | ||
* | ||
* @param action contains all the information to build a TodoTask. | ||
* @throws TodoException if the String action is invalid. | ||
*/ | ||
public TodoTasks(String action) throws TodoException { | ||
super(action, "[T][] ", TaskTypes.Todo); | ||
if (action.split(" ").length < 2){ | ||
throw new TodoException("OOPS!!! The description of a todo cannot be empty."); | ||
} | ||
} | ||
} | ||
|
||
class DeadlineTask extends Task{ | ||
/** | ||
* constructor for Task. | ||
* | ||
* @param action containinf all the data needed for the creation of Deadline Task. | ||
* @throws DeadlineException if the String action is invalid. | ||
*/ | ||
public DeadlineTask(String action) throws DeadlineException { | ||
super(action, "[D][] ", TaskTypes.Deadline); | ||
if (action.split("/").length < 2) { | ||
throw new DeadlineException("incorrect format for deadline task"); | ||
} | ||
if (action.split("/")[1].split(" ").length < 2) { | ||
throw new DeadlineException("incorrect date format for Deadline task"); | ||
} | ||
} | ||
} | ||
|
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.
Should make these classes in separate java files rather than within another file.
Improved GUI by add different colors to Duke and User DialogBox. Some error seemed to have occurred, reused previous local safed version to remove any errors.
Docs file was previously deleted by mistake
Amended user guide, partially done. Debugged code such that the bug whereby check duplicate function fails to work is resolved.
trial
smoke test done to test jar file validity
Improved syntax, removed the appearance of # in the UG
Ultimate Normal Scheduler
The Ultimate Normal Scheduler has one aim and one aim only, to make support you to be the ultimate Task Master!
text-based cause the GUI has not been done yet.
Features:
Add this into the code to run it!
public class Main { public static void main(String[] args) { Application.launch(MainApp.class, args); } }