Skip to content

[Adrian Ong] iP#302

Open
AdrianOngJJ wants to merge 45 commits into
nus-cs2103-AY2122S2:masterfrom
AdrianOngJJ:master
Open

[Adrian Ong] iP#302
AdrianOngJJ wants to merge 45 commits into
nus-cs2103-AY2122S2:masterfrom
AdrianOngJJ:master

Conversation

@AdrianOngJJ
Copy link
Copy Markdown

@AdrianOngJJ AdrianOngJJ commented Jan 29, 2022

LeDuke

FAST, LITE, POWERFUL MEGA STONKS
LeDuke will help you with:

  • Remembering tasks
  • Keeping track of which tasks are completed

What? Did I hear that you want LeDuke? Follow these easy steps 👀:

  1. Find code here
  2. Run the JAR file
  3. ...
  4. PROFIT!!

Progress:

  • Mabaging tasks
  • Managing deadlines
  • Reminders

You can also use look for the main in my code:

public static void main(String[] args) {
        new Duke("/tasks.txt").run();
    }

Copy link
Copy Markdown

@joshuayapwj98 joshuayapwj98 left a comment

Choose a reason for hiding this comment

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

Overall, an elegant and tidy set of code! Can distinguish the flow of the programme and it would be easy to take over as a public repo if ever you are going to publish this project 😄

Just some tiny pointers about the indentation and also whitespace! Otherwise, good job 👍🏼

Comment thread src/main/java/Duke.java Outdated
Comment on lines +33 to +38
for(int i = 0; i < masterList.size(); i++) {
Task curr = masterList.get(i);
bw.write((i + 1) + "." + curr.toString());
bw.newLine();
}
bw.flush();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A very intuitive set of code!

I was wondering if there's also a need for whitespace after the "for" word?

Nonetheless, neat work :)

Comment on lines +7 to +9
public DukeException(String message) {
super(message);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome that this class is kept simple!

However, maybe there should be some comments to document on what kind of exceptions you are expecting? 😄

Comment thread src/main/java/Duke.java Outdated
System.out.println("Hello! I'm Duke\nWhat can I do for you?");
String input; // to store raw input command
String[] inputArr; // to store split input command
boolean ifBye = false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Intuitive boolean to check if the program should exit or not!

However, maybe:

Suggested change
boolean ifBye = false;
boolean shouldAbort = false;

Copy link
Copy Markdown

@ovidharshini ovidharshini left a comment

Choose a reason for hiding this comment

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

LGTM after sorting out a few nits. Overall, the code adhered well to Java coding conventions. Spacing of comments and code appears to be irregular at times, so do keep a look out for them. You can also consider adding file types of ".class" to gitignore so as to not push them to remote.

Good job!

Comment thread src/main/java/Deadlines.java Outdated
* Deadlines tasks that need to be done before a specific date/time
* e.g., submit report by 11/10/2019 5pm
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider, perhaps, deleting this line break between the comments and the function for consistency

Comment thread src/main/java/Duke.java Outdated
private static final ArrayList<Task> masterList = new ArrayList<>();
/**
* Prints line break.
* @return void
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding a line break between the description of a function and its associated Javadoc tags for consistency. I noticed the same issue across other functions as well.

Comment thread src/main/java/Duke.java Outdated
private static final ArrayList<Task> masterList = new ArrayList<>();
/**
* Prints line break.
* @return void
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should the @return tag here be deleted? It is not required here as per Oracle guidelines since the return type is void. Do refer to Javadoc guidelines for a more thorough reference for the conventions with respect to Javadoc comments.

Comment thread src/main/java/Duke.java Outdated
Comment on lines +30 to +32
* @throws java.io.IOException If an I/O error occurs. Only takes in string.
*/
private static final void printList(BufferedWriter bw) throws Exception {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice job on catching this! Do consider throwing an explicit IOException if you mention it as such in the documentation for the code though

Comment thread src/main/java/Duke.java Outdated
* @throws java.io.IOException If an I/O error occurs. Only takes in string.
*/
private static final void printList(BufferedWriter bw) throws Exception {
for(int i = 0; i < masterList.size(); i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You might want to include a whitespace after the 'for' keyword here?

Comment thread src/main/java/Duke.java Outdated
}

private static final String getDateTime(String[] inputArr) {
return inputArr[1].split("/")[1].split(" ", 2)[1]; // split input from slash
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perhaps include the comment as a same-level indent above the line as opposed to an inline one? I noticed this in other places as well.

Since this function consists of a single line at the moment, you might want to add Javadoc-style documentation instead.

Comment thread src/main/java/Duke.java Outdated
Comment on lines +50 to +69
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(System.out));
String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
printLineBreak();
System.out.println("Hello! I'm Duke\nWhat can I do for you?");
String input; // to store raw input command
String[] inputArr; // to store split input command
boolean ifBye = false;
do {
printLineBreak();
System.out.println();
input = br.readLine();
inputArr = input.split(" ", 2); // split first word from body
printLineBreak();
switch (inputArr[0]) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do consider separating the code into logical blocks with line breaks for ease of viewing. For instance, here, the do-while loop can be separated from the rest of the preceding lines, and the initialization of variables (e.g. BufferedReader) can be separated in a similar fashion as well.

Comment thread src/main/java/Duke.java Outdated
Comment on lines +70 to +75
case "bye":
ifBye = true;
System.out.println("Bye. Hope to see you again soon!");
break;

case "list":
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good job on following the guidelines with respect to the indentation levels of switch and case! There should be no need for line breaks between individual case statements, so maybe remove these as a final step?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants