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

[Shulong] iP #467

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
3b19ba1
Add Gradle support
May 24, 2020
cc6c420
Level-1
Aug 29, 2020
f13b06c
Level-2
Aug 29, 2020
2fae636
Level-3
Aug 29, 2020
1492099
Level-4
Aug 29, 2020
da4753a
A-TextUiTesting
Aug 29, 2020
974e054
Level-5
Aug 29, 2020
8adb9f2
Level-6
Aug 29, 2020
0f7a149
Level-7
Aug 30, 2020
52f0b98
merge branch-level-7
Aug 30, 2020
02639bc
Level-8
Aug 30, 2020
df5b0b0
A-MoreOOP
Sep 2, 2020
17e5b38
A-JUnit
Sep 3, 2020
e9da59c
A-Jar
Sep 3, 2020
9e01d9f
A-JavaDoc
Sep 3, 2020
da712d0
A-CodingStandard
Sep 4, 2020
bdc38e7
Level-9
Sep 4, 2020
782a52b
A-JavaDoc
Sep 4, 2020
c0e5707
A-CodingStandard
Sep 4, 2020
258d6a3
merge branch-A-JavaDoc
Sep 4, 2020
9aed463
merge confilct
Sep 4, 2020
1cb9417
current version
Sep 4, 2020
d2a5a1c
merge gardle files
Sep 4, 2020
a842d79
fixed merge with gradle
Sep 4, 2020
56f75db
A-Gradle
Sep 4, 2020
89551fe
Level-10
Sep 8, 2020
3b5b52d
A-Assertions
Sep 8, 2020
20b255c
A-CodeQuality
Sep 8, 2020
479fca4
Merge pull request #1 from DreamerDragon/branch-A-Assertions
DreamerDragon Sep 9, 2020
ce7907f
updated master branch
Sep 9, 2020
a8122c7
Merge pull request #2 from DreamerDragon/branch-A-CodeQuality
DreamerDragon Sep 9, 2020
3bf5aa3
BCD-Extension
Sep 9, 2020
abe3628
Merge branch 'master' into C-findDuplicates
DreamerDragon Sep 9, 2020
9944c9e
Merge pull request #3 from DreamerDragon/C-findDuplicates
DreamerDragon Sep 9, 2020
928832c
A-BetterGui
Sep 16, 2020
173d7bb
A-UserGuide
Sep 16, 2020
e5a4502
A-betterGui
Sep 16, 2020
b6cd165
updates master branch with Parser and ParserGUI classes
Sep 16, 2020
6dab289
A-BetterGui
Sep 16, 2020
ccf2246
Set theme jekyll-theme-architect
DreamerDragon Sep 16, 2020
92c2ab3
Finalise KaTo
Sep 17, 2020
df831bb
add in docs/_config.yml
Sep 17, 2020
c40eb7a
Added clear command in ParserGui and GUI class
Sep 27, 2020
2080a0e
Update UG with the clear feature
Sep 27, 2020
6bf9cec
Added javaDoc for the clear command
Sep 27, 2020
0ce5803
Fixed import spacing issues
Sep 30, 2020
21dc446
removed clear command from the user guide
Oct 3, 2020
81272a5
removed clear command from the user guide
Oct 3, 2020
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
258 changes: 258 additions & 0 deletions src/main/java/Duke.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,268 @@
import java.util.ArrayList;

Choose a reason for hiding this comment

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

Hi, should there be a package statement above this line?
Reference: "Every class should be part of some package."

Copy link
Author

Choose a reason for hiding this comment

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

so how does your package look like?

Choose a reason for hiding this comment

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

it actually depends on how the directory in your project looks like. one of my package statements is "package command"

import java.util.Scanner;
import java.io.IOException;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;


public class Duke {
// Level 6
//for pr creation
// A-trial run
// program to be added
// Find out the position of the first '/'
public int firstSlashTracker (String string){

Choose a reason for hiding this comment

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

Names representing methods must be verbs and written in camelCase. For example, getName(), computeTotalWidth()

So, if the intention is for you to get the value of first slash tracker, the method name could be getFirstSlashTracker instead

int location = 0;
boolean exist = false;

Choose a reason for hiding this comment

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

Should the name of the boolean be "isExist"? I also noticed the same issue in other boolean variables too.
Reference: "Boolean variables/methods should be named to sound like booleans"

Copy link
Author

Choose a reason for hiding this comment

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

good point!

for (int i = 0; i < string.length(); i++){
if(string.charAt(i) == '/'){

Choose a reason for hiding this comment

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

Should there be whitespace after "if"? I noticed the same issue in several other places too.
Reference: "Java reserved words should be followed by a white space." from CS2103 Java coding standard web page

Copy link
Author

Choose a reason for hiding this comment

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

Thanks Dahfin, I will change that!

exist = true;
location = i;
break;
}
}
if(exist) {

Choose a reason for hiding this comment

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

There are some recurring inconsistencies with usage of white space. It might be good to review to keep it consistent with the rest of the code base.

for example: if (exist) would better adhere to coding styles than if(exist)

return location;
}else {
return -1;
}
}

private void run() {
Scanner sc = new Scanner(System.in);
String order = sc.nextLine();
ArrayList<Task> orders = new ArrayList<>();

//take in and process the order
while (!order.equals("bye")) {
var correct = true;
var deleted = false;
var done = order.length() >= 4 && order.substring(0 , 4).equals("done");
var delete = order.length() >= 6 && order.substring(0 , 6).equals("delete");
var todo = order.length() >= 4 && order.substring(0 , 4).equals("todo");
var deadline = order.length() >= 8 && order.substring(0, 8).equals("deadline");
var event = order.length() >= 5 && order.substring(0, 5).equals("event");
Copy link

Choose a reason for hiding this comment

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

Maybe you can consider specifying the data type of your variables instead of just using 'var' for all of them? I'm not sure about the technical benefits of using int, boolean, String etc but it would definitely improve the readability of your code which will be helpful for your tP! Also, it seems like all of them are boolean values so you could consider naming them isCorrect, isDeleted, isInputDone etc to follow the coding standard.

Copy link

Choose a reason for hiding this comment

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

Not sure if you already know but order.substring(0 , 4).equals("done") could be replaced with order.startsWith("done"), just to improve readability but you will get the same result either way.

if (order.equals("food left")) {
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Only an apple pie");
} else if (order.equals("allowance left")) {
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("I have checked, it is 20000 SGD");
} else if (order.equals("who is the most handsome guy")) {
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Of Course You!");
} else if (order.equals("task list")) {
System.out.println("Ka To:");
System.out.println(" ");
if(orders.size() == 0){
System.out.println(" You have not given me any task yet ");
}else {
for (int i = 0; i < orders.size(); i++) {
if(orders.get(i).status == 0) {
if(orders.get(i).category == 0){
System.out.println(i + 1 + ". " + " [T][x] "+ orders.get(i).command);
}else if(orders.get(i).category == 1){
System.out.println(i + 1 + ". " + " [D][x] " + orders.get(i).command);
}else if(orders.get(i).category == 2){
System.out.println(i + 1 + ". " + " [E][x] " + orders.get(i).command);
}else {
System.out.println(i + 1 + ". " + " [x] " + orders.get(i).command);
}
}else {
if(orders.get(i).category == 0){
System.out.println(i + 1 + ". " + " [T][v] "+ orders.get(i).command);
}else if(orders.get(i).category == 1){
System.out.println(i + 1 + ". " + " [D][v] " + orders.get(i).command);
}else if(orders.get(i).category == 2){
System.out.println(i + 1 + ". " + " [E][v] " + orders.get(i).command);
}else {
System.out.println(i + 1 + ". " + " [v] " + orders.get(i).command);
}
}
Copy link

Choose a reason for hiding this comment

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

One possible way to make the printing of the tasks out neater could be by overriding the toString method of your Task class so that you can just call System.out.println(i + 1 + ". " + orders.get(i)) instead. It is the same logic as what you are doing here, just that the checking of status and category is done inside the Task class and this will help with abstraction.

}
}
}else if (done) {
int number = Character.getNumericValue(order.charAt(5));
if(number > 0) {
orders.get(number - 1).status = 1;
}
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Wow, this is now solved! " + " [v] " + orders.get(number - 1).command);
}else if (delete) {

int number = Character.getNumericValue(order.charAt(7));
if(number > 0) {
if(orders.get(number - 1).status == 0){
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Noted, this is now deleted! " + " [x] " + orders.get(number - 1).command);
}else{
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Noted, this is now deleted! " + " [v] " + orders.get(number - 1).command);
}
orders.remove(number - 1);
deleted = true;
}

;
}else if(todo){
if(order.length() > 4){
System.out.println("Ka To:");
System.out.println();
System.out.println("Yup, this is now added! " + " [T][x] " + order.substring(4));
int size = orders.size() + 1;
System.out.println(" You have " + size + " task added now! ");
}
}
else if(deadline){
if(order.length() > 8){
System.out.println("Ka To:");
System.out.println();
int number = firstSlashTracker(order);
System.out.println("Yup, this is now added! " + " [D][x] " + order.substring(8, number) + "(by: " +
order.substring(number + 3) + ")");
int size = orders.size() + 1;
System.out.println(" You have " + size + " task added now! ");
}
}
else if(event){
if(order.length() > 5){
System.out.println("Ka To:");
System.out.println();
int number = firstSlashTracker(order);
System.out.println(" Yup, this is now added! " + " [E][x] " + order.substring(5, number) + "(at: " +
order.substring(number + 3) + ")");

Choose a reason for hiding this comment

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

It seems like you are making a line break in order to follow the rule of line length not exceeding 120 and that is great!

However, line breaks should be done before an operator. One possible way could be to change this line to the following format :
System.out.println(" Yup, this is now added! " + " [E][x] "

  • order.substring(5, number) + "(at: "
  • order.substring(number + 3) + ")");

the operator should be a part of the new line when wrapping lines. Hope this example clarifies 👍

Copy link
Author

Choose a reason for hiding this comment

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

Wow! Thank you for pointing it!!

int size = orders.size() + 1;
System.out.println(" You have " + size + " task added now! ");
}
}
else {
correct = false;
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("Sorry, I could not answer that");
}
if(correct && !done && !deleted && !order.equals("task list")) {
if(todo){
if(order.length() <= 4){
System.out.println("oops, the todo description cannot be empty");
}else {
Task item = new Task(0, 0, order.substring(4) );
orders.add(item);
}
}else if(deadline){
if(order.length() <= 8){
System.out.println("oops, the deadline description cannot be empty");
}else {
int number = firstSlashTracker(order);
Task item = new Task(1, 0,order.substring(8, number) + "(by: " +

Choose a reason for hiding this comment

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

Should the plus sign at the end of the line be placed below instead (i.e. after the break)?
Reference: "Break before an operator."

Copy link
Author

Choose a reason for hiding this comment

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

ohh I will take note !

order.substring(number + 3) + ")");
orders.add(item);
}
}else if(event){
if(order.length() <= 5){
System.out.println("oops, the event description cannot be empty");
}else {
int number = firstSlashTracker(order);
Task item = new Task(2, 0, order.substring(5, number) + "(at: " +
order.substring(number + 3) + ")");
orders.add(item);
}

}else {
Task item = new Task(4, 0, order);
orders.add(item);
}

}
Copy link

Choose a reason for hiding this comment

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

I'm not sure why you decided to make another if block to check for todo, event, and deadline since you already checked for the booleans at lines 117, 126 and 137. Would combining the bodies of the 2 if blocks of todo change the output in any way?

order = sc.nextLine();
}
System.out.println("Ka To:");
System.out.println(" ");
System.out.println("I am happy to serve you. See you soon!");
}

public static void main(String[] args) {
String filePath = "./data/duke.txt";

try {
File myFile = new File(filePath);
Scanner myReader = new Scanner(myFile);
while (myReader.hasNextLine()) {
String data = myReader.nextLine();
System.out.println(data);
}
myReader.close();
} catch (FileNotFoundException e) {
System.out.println("Creating the file to save");
try {
String folderPath = "./data";
Path path = Paths.get(folderPath);

Files.createDirectories(path);

System.out.println("Directory is created!");

//Files.createDirectory(path);

} catch (IOException error) {
System.err.println("Failed to create directory!" + error.getMessage());
}
try {
File fileCreator = new File(filePath);
if (fileCreator.createNewFile()) {
System.out.println("File created: " + fileCreator.getName());
} else {
System.out.println("File already exists.");
}
} catch (IOException ex) {
System.out.println("An error occurred.");
e.printStackTrace();
}
}
try {
File myFile = new File(filePath);
Scanner myReader = new Scanner(myFile);
while (myReader.hasNextLine()) {
String data = myReader.nextLine();
System.out.println(data);
}
myReader.close();
} catch (FileNotFoundException e) {
System.out.println("Creating the file to save");
}

try {
FileWriter myWriter = new FileWriter(filePath);
//myWriter.write("Files in Java might be tricky, but it is fun enough!");
myWriter.close();
System.out.println("All changes successfully saved to the storage!");
} catch (IOException e) {
System.out.println("An error occurred.");
e.printStackTrace();
}
Copy link

Choose a reason for hiding this comment

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

I'm sorry but I don't really understand this part. Would it be possible to combine all the try blocks together into one and have multiple catch blocks for different errors instead?

String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
System.out.println( "____________________________________________________________");
System.out.println("Hello :))! I'm your daily manager, Ka To! Welcome Back!");
System.out.println("how could I serve you now? ");
System.out.println("You could ask me any question if you like!");
System.out.println( "____________________________________________________________");
Duke manager = new Duke();
manager.run();
}

}
10 changes: 10 additions & 0 deletions src/main/java/Task.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
public class Task {
public int status;

Choose a reason for hiding this comment

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

Should this be declared as non-public? I also see the same issue on some other variables.
Reference: "Class variables should never be declared public unless the class is a data class with no behavior."

Copy link
Author

Choose a reason for hiding this comment

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

behaviour here means methods?

Choose a reason for hiding this comment

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

I'm not 100% sure, but I think it means methods

Copy link

@yuxuanxc yuxuanxc Sep 1, 2020

Choose a reason for hiding this comment

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

Does the status(0, 1) refer to whether or not the task is done? If that's the case, changing it to a boolean(eg. isDone) will be helpful in terms of readability. It might reduce confusion as your teammates will not have to guess which variable does what when they read your code for tP/ other group projects in the future.

public int category;
public String command;
Task(int category, int status, String command){
this.category = category;
this.status = status;
Copy link

Choose a reason for hiding this comment

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

The status of the task is always set to 0 when you initialize a new Task object, so the constructor may not have to take in a parameter for status and this.status = 0; could be used instead?

this.command = command;
}
}
67 changes: 65 additions & 2 deletions text-ui-test/EXPECTED.TXT
Original file line number Diff line number Diff line change
@@ -1,7 +1,70 @@
Hello from
____ _
| _ \ _ _| | _____
____ _
| _ \ _ _| | _____
| | | | | | | |/ / _ \
| |_| | |_| | < __/
|____/ \__,_|_|\_\___|

____________________________________________________________
Hello :))! I'm your daily manager, Ka To! Welcome Back!
how could I serve you now?
You could ask me any question if you like!
____________________________________________________________
oops, the todo description cannot be empty
oops, the deadline description cannot be empty
oops, the event description cannot be empty
Ka To:

Sorry, I could not answer that
Ka To:

You have not given me any task yet
Ka To:

Only an apple pie
Ka To:

I have checked, it is 20000 SGD
Ka To:

1. [x] food left
2. [x] allowance left
Ka To:

Yup, this is now added! [T][x] watch a movie
You have 3 task added now!
Ka To:

Yup, this is now added! [D][x] my homework (by: tonight)
You have 4 task added now!
Ka To:

Yup, this is now added! [E][x] meet my friend (at: lunchtime)
You have 5 task added now!
Ka To:

Wow, this is now solved! [v] allowance left
Ka To:

Wow, this is now solved! [v] my homework (by: tonight)
Ka To:

1. [x] food left
2. [v] allowance left
3. [T][x] watch a movie
4. [D][v] my homework (by: tonight)
5. [E][x] meet my friend (at: lunchtime)
Ka To:

Noted, this is now deleted! [v] allowance left
Ka To:

Noted, this is now deleted! [x] meet my friend (at: lunchtime)
Ka To:

1. [x] food left
2. [T][x] watch a movie
3. [D][v] my homework (by: tonight)
Ka To:

I am happy to serve you. See you soon!