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

[Adrian Ong] iP #302

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

Conversation

AdrianOngJJ
Copy link

@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

@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 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();

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);
}

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? 😄

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;

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

@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!

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

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

private static final ArrayList<Task> masterList = new ArrayList<>();
/**
* Prints line break.
* @return void

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.

private static final ArrayList<Task> masterList = new ArrayList<>();
/**
* Prints line break.
* @return void

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 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 {

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

* @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++) {

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?

}

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

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 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]) {

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 on lines 70 to 75
case "bye":
ifBye = true;
System.out.println("Bye. Hope to see you again soon!");
break;

case "list":

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