Skip to content
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

[Ming Gao Rickie Li] iP #469

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
016ad8d
Level 1. Greet, Echo, Exit
MGRL2201 Aug 18, 2021
d81e3b0
Level 2. Add, List
MGRL2201 Aug 18, 2021
87638eb
Level 3. Mark as Done
MGRL2201 Aug 18, 2021
7f141e7
Level 4. ToDos, Events, Deadlines
MGRL2201 Aug 18, 2021
ad65ad1
A-TextUiTesting: Automated Text UI Testing
MGRL2201 Aug 18, 2021
2a2efbd
Level 5. Handle Errors
MGRL2201 Aug 18, 2021
974d93a
Level 6. Delete
MGRL2201 Aug 18, 2021
b4b212f
A-Enums: Use Enums
MGRL2201 Aug 18, 2021
8a2b75e
Level 7. Save
MGRL2201 Aug 26, 2021
5927253
Level 7. Save
MGRL2201 Aug 26, 2021
344c259
Level 7. Save
MGRL2201 Aug 26, 2021
eddbc35
Level 8. Dates and Times
MGRL2201 Aug 26, 2021
e376a76
Merge branch 'branch-level-8'
MGRL2201 Aug 26, 2021
f334294
Use More OOP
MGRL2201 Aug 26, 2021
1e24a6c
Organize into Packages
MGRL2201 Aug 26, 2021
04d6408
Add JUnit Tests
MGRL2201 Aug 26, 2021
0e270b5
Create a JAR File
MGRL2201 Aug 26, 2021
bd59051
JavaDoc
MGRL2201 Aug 26, 2021
8ad2179
Follow the Coding Standard
MGRL2201 Aug 26, 2021
3369de3
Level 9. Find
MGRL2201 Aug 26, 2021
c12c5a7
A-Gradle
MGRL2201 Sep 2, 2021
2e99d62
A-Gradle
MGRL2201 Sep 2, 2021
22c64b4
A-Checkstyle
MGRL2201 Sep 2, 2021
ed5c92c
Level-10
MGRL2201 Sep 2, 2021
88616f7
No assertions statements.
MGRL2201 Sep 9, 2021
4155640
Merge pull request #1 from MGRL2201/branch-A-Assertions
MGRL2201 Sep 9, 2021
3a9ebac
Code quality is not great.
MGRL2201 Sep 9, 2021
3b2f505
Merge pull request #2 from MGRL2201/branch-A-CodeQuality
MGRL2201 Sep 9, 2021
472fac4
added update function
MGRL2201 Sep 9, 2021
07688eb
Merge pull request #3 from MGRL2201/BCD-Extension
MGRL2201 Sep 9, 2021
caf4dfa
polished code
MGRL2201 Sep 17, 2021
1a9c8fe
A-BetterGui
MGRL2201 Sep 19, 2021
d75e934
minor updates to commands
MGRL2201 Sep 20, 2021
91a4400
added assert messages
MGRL2201 Sep 20, 2021
341b156
Set theme jekyll-theme-tactile
MGRL2201 Sep 20, 2021
61841c2
Update README.md
MGRL2201 Sep 20, 2021
cafd45e
A-Release
MGRL2201 Sep 20, 2021
5716959
Merge branch 'master' of https://github.com/MGRL2201/ip
MGRL2201 Sep 20, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ bin/

/text-ui-test/ACTUAL.txt
text-ui-test/EXPECTED-UNIX.TXT

main.jar
4 changes: 4 additions & 0 deletions Data/taskList.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
T | 0 | borrow book
D | 0 | return book | Sunday
T | 0 | borrow book
T | 0 | return book
10 changes: 0 additions & 10 deletions src/main/java/Duke.java

This file was deleted.

26 changes: 26 additions & 0 deletions src/main/java/duke/Deadline.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package duke;
import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
public class Deadline extends Task {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Parser class? 🤔


protected String by;
protected LocalDate date;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these variables are not used in other parts of the code base consider changing the accessor to private.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider declaring variables as final if the attributes are not expected to change.


public Deadline(String description, String by) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Deadline method? 🤔
And I noticed the same issue in several other places below.

super(description);
this.by = by;
String[] d = by.split("-");
if (d.length == 3) {
date = LocalDate.parse(by);
}
}

@Override
public String toString() {
if (this.date != null) {
return "[D]" + super.toString() + " (by: " + this.date.format(DateTimeFormatter.ofPattern("MMM d yyyy")) + ")";
} else {
return "[D]" + super.toString() + " (by: " + by + ")";
}
}
}
37 changes: 37 additions & 0 deletions src/main/java/duke/Duke.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package duke;

import java.util.Scanner;

public class Duke {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Duke class? 🤔


private Storage storage;
private Ui ui;
private String filePath;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider declaring as final.


public Duke(String filePath) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Duke method? 🤔
And I noticed the same issue in several other places below.

this.ui = new Ui();
this.filePath = filePath;
this.storage = new Storage(filePath);
}

/**
* runs the Duke chat bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a full stop at the end? 🤔

*/
public void run() {
String separator = " ____________________________________________________________";
System.out.println(Ui.WelcomeMessage());
Scanner in = new Scanner(System.in);
String input = in.nextLine();
while(!Parser.parse(input).equals("bye")) {
this.storage.save(this.ui.run(Storage.load(), input), this.filePath);
input = in.nextLine();
}
System.out.println(separator);
System.out.println(" Bye. Hope to see you again soon!");
System.out.println(separator);
}

public static void main(String[] args) {
new Duke("C:\\Users\\Rickie\\Documents\\University\\Year 2\\Semester 1\\CS2103T\\ip\\Data\\taskList.txt").run();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be wrapped to next line? 🤔

}
}
54 changes: 54 additions & 0 deletions src/main/java/duke/DukeException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package duke;
public class DukeException extends RuntimeException{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a space between RuntimeException and { ? 🤔


public enum Type {
EmptyDone {
@Override
public String getMessage() {
return " ☹ OOPS!!! You haven't specified the task you have completed.";
}
},
EmptyDelete {
@Override
public String getMessage() {
return " ☹ OOPS!!! You haven't specified the task you want to delete.";
}
},
InvalidTaskNumber {
@Override
public String getMessage() {
return " ☹ OOPS!!! The task number you have specified is invalid.";
}
},
EmptyTodo {
@Override
public String getMessage() {
return " ☹ OOPS!!! The description of a todo cannot be empty.";
}
},
EmptyEvent {
@Override
public String getMessage() {
return " ☹ OOPS!!! The description of an event cannot be empty.";
}
},
EmptyDeadline {
@Override
public String getMessage() {
return " ☹ OOPS!!! The description of a deadline cannot be empty.";
}
},
InvalidCommand {
@Override
public String getMessage() {
return " ☹ OOPS!!! I'm sorry, but I don't know what that means :-(";
}
};

public abstract String getMessage();
}

public DukeException(Type type) {
super(type.getMessage());
}
}
26 changes: 26 additions & 0 deletions src/main/java/duke/Event.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package duke;
import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
public class Event extends Task {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Event class? 🤔


protected String at;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"at" is perhaps not descriptive enough

protected LocalDate date;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider changing to private final.


public Event(String description, String at) {
super(description);
this.at = at;
String[] d = at.split("-");
if (d.length == 3) {
date = LocalDate.parse(at);
}
}

@Override
public String toString() {
if (this.date != null) {
return "[E]" + super.toString() + " (at: " + this.date.format(DateTimeFormatter.ofPattern("MMM d yyyy")) + ")";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be wrapped to next line? 🤔

} else {
return "[E]" + super.toString() + " (at: " + this.at + ")";
}
}
}
33 changes: 33 additions & 0 deletions src/main/java/duke/Parser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package duke;
public class Parser {

/**
* Returns the command inputted by the user
*
* @param input
* @return command

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be more descriptive, eg. input from user cli / command to be executed by duke.

*/
public static String parse(String input) {
String inputLower = input.toLowerCase();
if (input.replaceAll("\\s","").toLowerCase().equals("bye")) {
return "bye";
} else if (inputLower.replaceAll("\\s", "").equals("list")) {
return "list";
} else if (inputLower.length() > 3 && inputLower.substring(0, 4).equals("done")) {
return "done";
} else if (inputLower.length() > 5 && inputLower.substring(0, 6).equals("delete")) {
return "delete";
} else if (inputLower.length() >= 4 && inputLower.substring(0, 4).equals("todo")) {
return "todo";
} else if (inputLower.length() >= 5 && inputLower.substring(0, 5).equals("event")) {
return "event";
} else if (inputLower.length() >= 8 && inputLower.substring(0, 8).equals("deadline")) {
return "deadline";
} else if (inputLower.length() >=4 && inputLower.substring(0, 4).equals("find")) {
return "find";
} else {
return "InvalidCommand";
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using case switch. Additionally, consider implementing a general remove first word method that extracts the string before the first space.

}

}
90 changes: 90 additions & 0 deletions src/main/java/duke/Storage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package duke;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Scanner;

public class Storage {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Storage class? 🤔


protected String filePath;
protected static File taskList;

public Storage(String filePath) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Storage method? 🤔
And I noticed the same issue in several other places below.

this.filePath = filePath;
this.taskList = new File(filePath);
}

/**
* Returns user's list of tasks
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a full stop at the end? 🤔

*
* @return Task List
*/
public static ArrayList<Task> load() {
ArrayList<Task> list = new ArrayList<>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps name the list as tasks ? 🤔

try {
if (!taskList.createNewFile()) {
Scanner fileReader = new Scanner(taskList);
while (fileReader.hasNextLine()) {
String line = fileReader.nextLine();
String[] splitString = line.split(" \\| ");
if (splitString[0].equals("T")) {
Todo t = new Todo(line.substring(8));
if (line.charAt(4) == '1') {
t.markAsDone();
}
list.add(t);
} else if (splitString[0].equals("D")) {
Deadline d = new Deadline(splitString[2], splitString[3]);
if (splitString[1].equals("1")) {
d.markAsDone();
}
list.add(d);
} else {
Event e = new Event(splitString[2], splitString[3]);
if (splitString[1].equals("1")) {
e.markAsDone();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could perhaps be SLAP-ed harder?

}
list.add(e);
}
}
fileReader.close();
}
} catch (IOException e) {
System.out.println("An error occurred.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider specifying the type of error. eg. error loading tasks.

e.printStackTrace();
}
return list;
}

public static void save(ArrayList<Task> list, String filePath) {
File taskList = new File(filePath);
taskList.delete();
try {
FileWriter writer = new FileWriter(filePath);
for (int i = 0; i < list.size(); i++) {
File tasks = new File(filePath);
Task t = list.get(i);
String isDone = t.isDone ? "1" : "0";
if (t instanceof Todo) {
String line = "T | " + isDone + " | " + t.description;
writer.write(line + "\n");
} else if (t instanceof Deadline) {
Deadline d = (Deadline) t;
String line = "D | " + isDone + " | " + t.description + " | " + d.by;
writer.write(line + "\n");
} else if (t instanceof Event) {
Event e = (Event) t;
String line = "E | " + isDone + " | " + t.description + " | " + e.at;
writer.write(line + "\n");
}
}
writer.close();
} catch (IOException e) {
System.out.println("An error occurred.");
e.printStackTrace();
}
}

}
23 changes: 23 additions & 0 deletions src/main/java/duke/Task.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package duke;
public class Task {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Task class? 🤔

protected String description;
protected boolean isDone;

public Task(String description) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Task method? 🤔
And I noticed the same issue in several other places below.

this.description = description;
this.isDone = false;
}

public String getStatusIcon() {
return (isDone ? "X" : " ");
}

public void markAsDone() {
this.isDone = true;
}

@Override
public String toString() {
return "[" + this.getStatusIcon() + "] " + this.description;
}
}
12 changes: 12 additions & 0 deletions src/main/java/duke/Todo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package duke;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style issue here, but there should always be a new line after package statement

public class Todo extends Task {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Todo class? 🤔


public Todo(String description) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps need a descriptive header comments for the Todo method? 🤔
And I noticed the same issue in several other places below.

super(description);
}

@Override
public String toString() {
return "[T]" + super.toString();
}
}
Loading