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

[Pranav Ganesh] iP #477

Open
wants to merge 51 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
d839859
Add Gradle support
May 24, 2020
244ea63
Finished Level-1
e0661424 Aug 19, 2021
05b4e5e
Finished Level-2
e0661424 Aug 19, 2021
bad6e71
Finished Level-3
e0661424 Aug 19, 2021
15cfb00
Finished Level-4
e0661424 Aug 19, 2021
6a1c9de
Finished A-TextUiTesting
e0661424 Aug 19, 2021
667a5bd
Finished Level-5
e0661424 Aug 19, 2021
57c60d1
Finished Level-6
e0661424 Aug 19, 2021
e70d0aa
Comply with Java coding standard
e0661424 Aug 26, 2021
80299be
Finish Level-7
e0661424 Aug 27, 2021
cfad495
Finish Level-8
e0661424 Aug 27, 2021
10b01bb
Add line wrapping at appropriate places
e0661424 Aug 27, 2021
2c44649
Merge branch 'branch-Level-7'
e0661424 Aug 28, 2021
f7e6d0a
Merge branch branch-Level-8 into Master
e0661424 Aug 28, 2021
7ae663c
Add new files for individual classes in Duke.java
e0661424 Aug 28, 2021
c6035cd
Add TaskList class
e0661424 Aug 28, 2021
a5f3c25
Add Storage class
e0661424 Aug 28, 2021
8026b89
Add Command class and relevant subclasses
e0661424 Aug 28, 2021
9605358
Enhance the functionality of the Storage class
e0661424 Aug 28, 2021
4537188
Add Ui class
e0661424 Aug 28, 2021
068f340
Finish A-MoreOOP
e0661424 Aug 28, 2021
493460f
Add A-Packages
e0661424 Aug 29, 2021
8479f34
Add JUnit Test for the Parser class
e0661424 Aug 29, 2021
02e7e75
Add JUnit Test for the DukeException class
e0661424 Aug 29, 2021
8381e30
Add JUnit Test for the TaskList class
e0661424 Aug 29, 2021
be2314a
Finish A-JUnit
e0661424 Aug 29, 2021
ce9da73
Modify .gitignore file
e0661424 Aug 31, 2021
1e3a89f
Finish A-CodingStandard
e0661424 Sep 1, 2021
92529e0
Finish A-JavaDoc
e0661424 Sep 1, 2021
11a840b
Finish Level-9
e0661424 Sep 1, 2021
15859ea
Merge branch 'branch-A-JavaDoc'
e0661424 Sep 1, 2021
528b73e
Merge branch-A-CodingStandard into Master
e0661424 Sep 1, 2021
8621cd4
Merge branch 'branch-Level-9'
e0661424 Sep 1, 2021
24169f9
Finish Level-9 (Latest)
e0661424 Sep 1, 2021
1e26b45
Merge remote-tracking branch 'origin/add-gradle-support'
e0661424 Sep 2, 2021
013d465
Modify project directory structure
e0661424 Sep 2, 2021
f01545c
Add branch-Level-10
e0661424 Sep 9, 2021
a84441a
Merge branch 'branch-Level-10'
e0661424 Sep 9, 2021
c2dfd54
Finish Level-10
e0661424 Sep 9, 2021
815d295
Enhance exception handling
e0661424 Sep 10, 2021
690a8f4
Add assertions to document important assumptions in the code
e0661424 Sep 13, 2021
956c0a1
Merge branch-A-Assertions into master
pranav-ganesh Sep 13, 2021
d8dfd36
Refactor code to improve code quality
e0661424 Sep 15, 2021
bf16d73
Merge branch-A-CodeQuality into master
pranav-ganesh Sep 15, 2021
c2cbb49
Add C-Update
e0661424 Sep 20, 2021
8afc4ce
Add Ui.png and enhance GUI
e0661424 Sep 20, 2021
2283f09
Add User Guide
e0661424 Sep 20, 2021
b93000a
Add Javadoc to more files
e0661424 Sep 20, 2021
5802939
Add final touches to the code
e0661424 Sep 20, 2021
3848c56
Update User Guide
e0661424 Sep 20, 2021
7668565
Further Update User Guide
e0661424 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/.idea/
/out/
/*.iml
out/artifacts/ip_jar

# Gradle build files
/.gradle/
Expand Down
Empty file added duke.txt
Empty file.
41 changes: 34 additions & 7 deletions src/main/java/Duke.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,37 @@
import duke.Storage;
import duke.task.TaskList;
import duke.Ui;
import duke.Parser;
import duke.command.Command;

public class Duke {
private Storage storage;
private TaskList taskList;
private Ui ui;

public Duke(String filePath) {
taskList = new TaskList();
storage = new Storage(filePath);
ui = new Ui(taskList, storage);
}

public void run() {
ui.showWelcome();
ui.showLine();
boolean isExit = false;
while (!isExit) {
String command = ui.command();
Parser parser = new Parser(command);
if (parser.isExit()) {
break;
}
Command c = parser.parse();

Choose a reason for hiding this comment

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

You can consider making parse() a static method, that way you would not have to create a new parser object everytime a command is given.

c.execute(taskList, storage);
isExit = parser.isExit();
}
}

public static void main(String[] args) {
String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
new Duke("./duke.txt").run();
}
}
}
3 changes: 3 additions & 0 deletions src/main/java/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Manifest-Version: 1.0
Main-Class: Duke

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

import duke.command.*;

Choose a reason for hiding this comment

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

You might want to avoid wild card imports even if you have to import everything
https://se-education.org/guides/conventions/java/intermediate.html#package-and-import-statements

Copy link

@yongxiangng yongxiangng Aug 30, 2021

Choose a reason for hiding this comment

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

For the parse method below

Perhaps a switch statement here would help to improve readability or you could modify the code by splitting the string by the empty character delimiter twice so you don't have to do another if inside the external if

Maybe your InvalidCommand can be an exception, and your doneCommand can throw the exception so you don't have to do all the checking within this method which could make the code easier to read

https://www.geeksforgeeks.org/split-string-java-examples/

https://nus-cs2103-ay2122s1.github.io/website/se-book-adapted/chapters/codeQuality.html#guideline-maximise-readability


public class Parser {
private String command;
Copy link

Choose a reason for hiding this comment

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

The string name command can be confusing, because a class called Command is in this project, but command is not a Command object.

Maybe change it to commandString?


public Parser(String command) {
this.command = command;
}

public Command parse() {

Choose a reason for hiding this comment

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

Perhaps try not to write deeply nested loops in your function? You might consider using 'switch' for this function?☺️

if (command.equals("list")) {
return new listCommand(command);
} else if (command.startsWith("done") && Character.isDigit(command.charAt(command.length() - 1))
&& command.length() <= 8 && !Character.isAlphabetic(command.charAt(command.length() - 2))
&& Character.isDigit(command.charAt(5))) {
return new doneCommand(command);
} else {
if (command.startsWith("todo")) {
return new todoCommand(command);
} else if (command.startsWith("deadline")) {
return new deadlineCommand(command);
} else if (command.startsWith("event")) {
return new eventCommand(command);
} else if (command.startsWith("delete")) {
return new deleteCommand(command);
} else {
return new InvalidCommand(command);

Choose a reason for hiding this comment

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

You can consider using a switch statement here 😄

}
}
}

public boolean isExit() {
if (command.equals("bye")) {
Ui.bye();
Ui.showLine();
return true;
}
if (command.equals("")) {
return true;
}
return false;
}

}














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

import org.junit.Test;
import org.junit.Assert;

public class ParserTest {
@Test
public void isExit_commandBye_true() {
Assert.assertEquals(true, new Parser("bye").isExit());
}

@Test
public void isExit_commandEmptyString_true() {
Assert.assertEquals(true, new Parser("").isExit());
}

@Test
public void isExit_commandDeadline_false() {
Assert.assertEquals(false, new Parser("deadline book ticket/by 2016-09-22 1700").isExit());
}

@Test
public void parse_commandDeadline_success() {
Assert.assertEquals("This is a deadline command", new Parser("deadline book ticket/by 2016-09-22 1700").parse().toString());
}

@Test
public void parse_commandEvent_success() {
Assert.assertEquals("This is an event command", new Parser("event concert/at 2016-09-22 1900").parse().toString());
}

@Test
public void parse_commandTodo_success() {
Assert.assertEquals("This is a todo command", new Parser("todo play cricket").parse().toString());
}

@Test
public void parse_commandDelete_success() {
Assert.assertEquals("This is a delete command", new Parser("delete 1").parse().toString());
}

@Test
public void parse_commandDone_success() {
Assert.assertEquals("This is a done command", new Parser("done 1").parse().toString());
}

@Test
public void parse_commandInvalidString_success() {
Assert.assertEquals("This is an invalid command", new Parser("Some invalid string").parse().toString());
}
}
84 changes: 84 additions & 0 deletions src/main/java/duke/Storage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package duke;

import duke.task.*;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileWriter;
import java.io.IOException;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Scanner;

public class Storage {
private String filepath;
private File file;

public Storage(String filepath) {
this.filepath = filepath;
try {
File file = new File(filepath);
if (!file.exists()) {
file.createNewFile();
}
this.file = file;
} catch (IOException e) {
System.out.println("Something went wrong: " + e.getMessage());
}
}

public void loadTaskListData(TaskList taskList) {

Choose a reason for hiding this comment

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

You may want to document public methods with javadocs
https://se-education.org/guides/conventions/java/intermediate.html#comments

The method here is also quite lengthy. It might be a good idea to abstract the other logic into various methods
https://nus-cs2103-ay2122s1.github.io/website/se-book-adapted/chapters/codeQuality.html#avoid-long-methods

Choose a reason for hiding this comment

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

You might want to break it down into smaller functions here? This function seems to be too deeply nested.

try {
Scanner s = new Scanner(this.file); // create a Scanner using the File as the source
if (!s.hasNext()) {

Choose a reason for hiding this comment

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

The logic here is quite deeply nested, perhaps it might be good if you handled the unusual case with a guard clause
https://nus-cs2103-ay2122s1.github.io/website/se-book-adapted/chapters/codeQuality.html#make-the-happy-path-prominent

if (!s.hasNext()) {
    // code here...
    return;
}

Ui.printMessage("There are no items in your task list!");
} else {
Ui.printMessage("Here is your current task list: ");
while (s.hasNext()) {
String str = s.nextLine();
System.out.println(str);
String[] parts = str.split("\\|", 4);
String subStr = parts[0].substring(3).trim();
if (subStr.equals("duke.task.Todo") || subStr.equals("duke.task.Todo")) {

Choose a reason for hiding this comment

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

I might be wrong here but doesn't subStr.equals("duke.task.Todo") || subStr.equals("duke.task.Todo") do the same as subStr.equals("duke.task.Todo")

Task task = new Todo(parts[2].trim());
if (parts[1].trim().equals("X")) {
task.markAsDone();
}
taskList.addTask(task);
} else if (subStr.equals("duke.task.Deadline") || subStr.equals("duke.task.Deadline")) {
DateTimeFormatter dtf = DateTimeFormatter.ofPattern("MMM d yyyy, h a");
LocalDateTime dateTime = LocalDateTime.parse(parts[3].substring(5).trim(), dtf);
Task task = new Deadline(parts[2].trim(), dateTime);
if (parts[1].trim().equals("X")) {
task.markAsDone();
}
taskList.addTask(task);
} else {

Choose a reason for hiding this comment

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

You may want to use a switch statement for this long if else. It may be good because you're comparing strings and it can directly be used for your cases.

Additionally the else statement here captures all other cases of subStr, not necessarily when it is an Event. If new requirements come in and there are new type of Task, the developer might forget to update the logic here and the new type of Task will automatically fall into the else case and treated like an Event.

The default case in a switch statement could be used to catch invalid cases. I believe the default case is optional in Java
https://www.geeksforgeeks.org/switch-statement-in-java/

The style guide suggests that

Always include a default branch in case statements.

Furthermore, use it for the intended default action and not just to execute the last option. If there is no default action, you can use the default branch to detect errors (i.e. if execution reached the default branch, raise a suitable error). This also applies to the final else of an if-else construct. That is, the final else should mean 'everything else', and not the final option. Do not use else when an if condition can be explicitly specified, unless there is absolutely no other possibility.

DateTimeFormatter dtf = DateTimeFormatter.ofPattern("MMM d yyyy, h a");
LocalDateTime dateTime = LocalDateTime.parse(parts[3].substring(5).trim(), dtf);
Task task = new Event(parts[2].trim(), dateTime);
if (parts[1].trim().equals("X")) {
task.markAsDone();
}
taskList.addTask(task);
}
}
Ui.printMessage("End of task list");
}
} catch (FileNotFoundException e) {
System.out.println("File not found!");
}
}

public void writeToFile(String filePath, TaskList tl) {
try {
FileWriter fw = new FileWriter(filePath);
for (int i = 0; i < tl.size(); i++) {
int num = i + 1;
fw.write(num + ". " + tl.getTask(i).taskListOnDisk() + "\n");
}
fw.close();
} catch (IOException e) {
System.out.println("Something went wrong: " + e.getMessage());
}
}
}
74 changes: 74 additions & 0 deletions src/main/java/duke/Ui.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package duke;

import duke.task.Task;
import duke.task.TaskList;
import java.util.Scanner;

public class Ui {
Scanner sc;
private static TaskList taskList;
private static Storage storage;
Comment on lines +13 to +14
Copy link

Choose a reason for hiding this comment

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

Why are taskList and storage static?


public Ui(TaskList taskList, Storage storage) {
sc = new Scanner(System.in);
this.taskList = taskList;
this.storage = storage;
}

public String command() {
if (sc.hasNextLine()) {
String str = sc.nextLine();
return str;
}
return "";
}

public void showWelcome() {
System.out.println("Hello from Duke!");
System.out.println("");
storage.loadTaskListData(taskList);
System.out.println("");
System.out.println("Hope you are doing well. How can I help you?");
}

public static void bye() {
System.out.println("Bye. Have a great day!");
}

public static void showLine() {
System.out.println("-----------------------------------------------");
}

public static void printMessage(String msg) {
System.out.println(msg);
}

public static void taskResponse(Task task) {
System.out.println("Got it. I've added this task:");
System.out.println(task);
System.out.println("Now you have " + taskList.size() + " tasks in the list.");
}

public static void doneResponse(Task task) {
System.out.println("Nice! I've marked this task as done:");
System.out.println(task);
storage.writeToFile("./duke.txt", taskList);
}

Choose a reason for hiding this comment

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

Good idea to write to file everytime a task is edited. This way the state of the tasks are not lost incase the app crashes.


public static void deleteResponse(Task task) {
System.out.println("Noted. I've removed this task:");
System.out.println(task);
System.out.println("Now you have " + taskList.size() + " tasks in the list.");
}
}











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

import duke.Storage;
import duke.task.TaskList;

public abstract class Command {
private String command;
Copy link

Choose a reason for hiding this comment

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

Perhaps consider making command protected instead of private? It would make the code much cleaner since subclasses make use of this variable a lot


public Command(String command) {
this.command = command;
}

public abstract void execute(TaskList taskList, Storage storage);
}
24 changes: 24 additions & 0 deletions src/main/java/duke/command/InvalidCommand.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package duke.command;

import duke.exception.DukeException;
import duke.exception.InvalidCommandException;
import duke.Storage;
import duke.task.TaskList;

public class InvalidCommand extends Command {
private String command;

public InvalidCommand(String command) {
super(command);
this.command = command;
}

public String toString() {
return "This is an invalid command";
}

public void execute(TaskList taskList, Storage storage) {
Copy link

Choose a reason for hiding this comment

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

Should have @Override annotation

DukeException exp = new InvalidCommandException("OOPS!!! I'm sorry, but I don't know what that means :-(");
System.out.println(exp);
}
}
Loading