-
Notifications
You must be signed in to change notification settings - Fork 438
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
base: master
Are you sure you want to change the base?
[Shulong] iP #467
Changes from 7 commits
3b19ba1
cc6c420
f13b06c
2fae636
1492099
da4753a
974e054
8adb9f2
0f7a149
52f0b98
02639bc
df5b0b0
17e5b38
e9da59c
9e01d9f
da712d0
bdc38e7
782a52b
c0e5707
258d6a3
9aed463
1cb9417
d2a5a1c
a842d79
56f75db
89551fe
3b5b52d
20b255c
479fca4
ce7907f
a8122c7
3bf5aa3
abe3628
9944c9e
928832c
173d7bb
e5a4502
b6cd165
6dab289
ccf2246
92c2ab3
df831bb
c40eb7a
2080a0e
6bf9cec
0ce5803
21dc446
81272a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,268 @@ | ||
import java.util.ArrayList; | ||
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){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) == '/'){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) + ")"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 :
the operator should be a part of the new line when wrapping lines. Hope this example clarifies 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
public class Task { | ||
public int status; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. behaviour here means methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure, but I think it means methods There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
} |
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! |
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.
Hi, should there be a package statement above this line?
Reference: "Every class should be part of some package."
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.
so how does your package look like?
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 actually depends on how the directory in your project looks like. one of my package statements is "package command"