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

[Nguyen Hoang Hai Minh] iP #445

Open
wants to merge 71 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
3b19ba1
Add Gradle support
May 24, 2020
cb0dcf9
Implement Level-1
nevirmc Aug 20, 2020
ff00562
Level-2
nevirmc Aug 20, 2020
299005e
Level-3
nevirmc Aug 20, 2020
052071c
Level-3
nevirmc Aug 20, 2020
d53eb38
Level-4
nevirmc Aug 20, 2020
c9c67ac
TextUiTesting
nevirmc Aug 20, 2020
7345f13
Level-5
nevirmc Aug 20, 2020
61ace1f
Level-6
nevirmc Aug 20, 2020
828bf0b
level 7 added
nevirmc Aug 27, 2020
87a8790
no message
nevirmc Aug 27, 2020
eff84ea
level 8 added
nevirmc Aug 27, 2020
716d3c3
Merge branch 'branch-Level-8'
nevirmc Aug 27, 2020
a9a8581
level-8 added
nevirmc Aug 27, 2020
9e2f0f5
Level-8 fixed
nevirmc Aug 27, 2020
12fe9af
A-Packages added
nevirmc Aug 27, 2020
758241f
A-JUnit added
nevirmc Aug 27, 2020
367da21
no message
nevirmc Aug 27, 2020
d47603e
A-JavaDoc added
nevirmc Aug 27, 2020
3f6b540
Level-9 Added
nevirmc Aug 27, 2020
53a20a7
Merge branch 'branch-Level-9'
nevirmc Aug 27, 2020
e5257ac
Fix package
nevirmc Sep 2, 2020
f71b3c9
no message
nevirmc Sep 2, 2020
d5b4965
Merge branch 'add-gradle-support' of https://github.com/minhhhnguyen2…
nevirmc Sep 2, 2020
af6a803
add GUI
nevirmc Sep 2, 2020
ca6ec8c
no message
nevirmc Sep 3, 2020
7428cf4
no message
nevirmc Sep 3, 2020
8e6c851
no message
nevirmc Sep 10, 2020
d3b8993
Add Assert
nevirmc Sep 10, 2020
396f602
Fix Code Quality
nevirmc Sep 10, 2020
9372042
Use lambda to print
nevirmc Sep 10, 2020
8803587
Change lambda expression
nevirmc Sep 10, 2020
25a54ce
Add Stream Collectors
nevirmc Sep 10, 2020
23a3044
Merge pull request #4 from minhhhnguyen2000/branch-A-CodeQuality
nevirmc Sep 10, 2020
9e14f33
Merge branch 'master' into branch-A-Assertions
nevirmc Sep 10, 2020
d2cc0a8
Merge branch 'master' into branch-A-Lambdas
nevirmc Sep 10, 2020
208bff7
Merge branch 'master' into branch-A-Streams
nevirmc Sep 10, 2020
8f99e1d
Merge pull request #3 from minhhhnguyen2000/branch-A-Lambdas
nevirmc Sep 10, 2020
44ec70b
Merge branch 'master' into branch-A-Streams
nevirmc Sep 10, 2020
e1a5959
Merge branch 'master' into branch-A-Assertions
nevirmc Sep 10, 2020
a5219a1
Merge pull request #2 from minhhhnguyen2000/branch-A-Streams
nevirmc Sep 10, 2020
2b4f16d
Merge branch 'master' into branch-A-Assertions
nevirmc Sep 10, 2020
29bd1ae
Merge pull request #1 from minhhhnguyen2000/branch-A-Assertions
nevirmc Sep 10, 2020
e393d3d
no message
nevirmc Sep 10, 2020
692c58e
Create gradle.yml
nevirmc Sep 10, 2020
d4b0d9d
Add dowithin
nevirmc Sep 10, 2020
6e5ad76
no message
nevirmc Sep 15, 2020
e8b9524
no message
nevirmc Sep 15, 2020
00f4ff0
no message
nevirmc Sep 15, 2020
426b401
Change to better OOP / Code Quality / JavaDoc
nevirmc Sep 15, 2020
f9481a7
no message
nevirmc Sep 16, 2020
0cef394
Divide into multiple packages
nevirmc Sep 16, 2020
23525e3
Change Task.java to abstract class
nevirmc Sep 16, 2020
407b183
better UI
nevirmc Sep 17, 2020
462aaa7
no message
nevirmc Sep 17, 2020
28b3deb
fix CheckStyle to detect coding style violations
nevirmc Sep 17, 2020
09a96b7
seperate DateFormatter
nevirmc Sep 17, 2020
1f73d1d
seperate DateFormatter
nevirmc Sep 17, 2020
2e39786
no message
nevirmc Sep 17, 2020
befc6ad
added product picture
nevirmc Sep 17, 2020
c2d3a3d
Fix Ui.png
nevirmc Sep 17, 2020
c24f512
Better OOP and Packages
nevirmc Sep 17, 2020
b28e166
update README.md
nevirmc Sep 17, 2020
779eb86
Set theme jekyll-theme-cayman
nevirmc Sep 17, 2020
e88c219
updates README.md
nevirmc Sep 17, 2020
601725b
Merge branch 'master' of https://github.com/minhhhnguyen2000/ip
nevirmc Sep 17, 2020
012b4f6
no message
nevirmc Sep 17, 2020
ce880a7
no message
nevirmc Sep 17, 2020
1ed4376
no message
nevirmc Sep 18, 2020
08f971a
Fix some minor JavaDoc error and updated build.gradle
nevirmc Sep 18, 2020
18e4cee
Update UI because of bug with jar
nevirmc Sep 18, 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
11 changes: 11 additions & 0 deletions data/duke.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
10
T|read book|false
T|read book|true
T|return book /by June 6|false
D|return book|June 6|true
E|dinner|6 30|false
T|cak|false
T|sleep|false
D|sleep|6|false
D|sleep|Oct 15 2019|false
D|sle|23|false
41 changes: 41 additions & 0 deletions src/main/java/Deadline.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//package duke;
/**
* Deadline <- Task
*/

Choose a reason for hiding this comment

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

Is there a better way to describe the Deadline class in the Javadoc comment? I understand what you are trying to convey but perhaps it would be better to elaborate on it! I personally prefer this representation as its very concise however it might not be intuitive enough for other readers.

public class Deadline extends Task {

Choose a reason for hiding this comment

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

I like how all the class names in your code and clear, concise and adhered to the Java coding standards and naming conventions.

String description;
String deadline;
boolean isDone;

Choose a reason for hiding this comment

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

Should these class variables be declared with a non-public access modifier to adhere to encapsulation principles? Perhaps you could consider using protected or private?


/**
* Init Deadline
*/
public Deadline(String description, String deadline) {
super(description);
this.deadline = deadline;
}

/**
* Status in format [D][x] return book (by: 6 June)
*/
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Perhaps you could consider following the guidelines for Java coding standards when writing JavaDoc comments? I think you are missing a 'return parameter' for the JavaDoc comment, or perhaps is there any reason why you wrote the JavaDoc comments in the way you did? I observed the same issue in several other methods as well.

@Override
public String getStatus() {
return "[D]" + super.getStatus() + " (by: " + deadline + ")";
}

/**
* type of Task -> D for deadline
*/
@Override
public String getType() {
return "D";
}

/**
* description to write to data file
*/
@Override
public String getDescription() {
return super.getDescription() + "|" + this.deadline;
}
}
154 changes: 148 additions & 6 deletions src/main/java/Duke.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,152 @@
//package duke;
import java.io.IOException;
import java.util.*;
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Perhaps you could list out all the imported classes explicitly? I noticed the same issue in several other classes as well.

import java.io.File;
import java.io.FileWriter;
import java.io.FileNotFoundException;
import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
/**
* Duke the best chatbot hehe
*/
public class Duke {

private Storage storage;
private TaskList tasks;
private Ui ui;
private Parser parser;

Choose a reason for hiding this comment

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

Perhaps you could consider adding JavaDoc comments for class members as well?


/**
* Init Duke
*/
public Duke() {
ui = new Ui();
storage = new Storage();
tasks = new TaskList(storage.readData());
parser = new Parser();

Choose a reason for hiding this comment

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

I like this way of initialising the variables as its very concise. However, perhaps you could consider using "this" to refer to the instance variables? I feel like it would be a more explicit representation of the initialisation in the constructor.

}

/**
* Main
*/
public static void main(String[] args) {
String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
new Duke().run();
}

/**
* Take in command
*/
public void run() {
ui.sayHi();
Scanner myScanner = new Scanner(System.in);
while(true) {
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Perhaps you could add a whitespace after 'while'? The guidelines on Java coding standards states that Java reserved words should be followed by a white space. I noticed this issue in other methods containing while, if and if-else blocks as well.

String cmd = myScanner.nextLine();
if(cmd.equals("bye")) {
ui.sayBye();
break;
}
else if(cmd.equals("list")) {
System.out.println("Here are the tasks in your list:");
for(int i = 1; i <= tasks.getSize(); ++i) {
System.out.println(i + "." + tasks.get(i - 1).getStatus());
}
}
else if(cmd.length() >= 4 && cmd.substring(0, 4).equals("done")) {

Choose a reason for hiding this comment

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

Perhaps you could use comments to further elaborate on your "else if" statements' condition? I feel like it would definitely be clearer and will increase readability for readers!

int c = 0;
for(int i = 5; i < cmd.length(); ++i) {
c = c * 10 + cmd.charAt(i) - '0';
}
System.out.println("Nice! I've marked this task as done:");
tasks.get(c - 1).done();
System.out.println(tasks.get(c - 1).getStatus());
}
else if(cmd.length() >= 4 && cmd.substring(0, 4).equals("todo")) {
try {
checkCmd(cmd, 4, "☹ OOPS!!! The description of a todo cannot be empty.");
String getName = cmd.substring(5);
Todo tmp = new Todo(getName);
System.out.println("Got it. I've added this task: ");
System.out.println(" " + tmp.getStatus());
tasks.add(tmp);
System.out.println("Now you have " + tasks.getSize() + " tasks in the list.");
}
catch (DukeException ex) {
System.out.println(ex.getMessage());
}
}
else if(cmd.length() >= 8 &&cmd.substring(0, 8).equals("deadline")) {
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Should there be an extra whitespace after the '&&' operator? I noticed the same issue in other areas as well.

try {
checkCmd(cmd, 8, "☹ OOPS!!! The description of a deadline cannot be empty.");
String getName = parser.getNameBy(cmd);
String getDeadline = parser.getDeadlineBy(cmd);
getDeadline = formatDate(getDeadline);
Deadline tmp = new Deadline(getName, getDeadline);
System.out.println("Got it. I've added this task: ");
System.out.println(" " + tmp.getStatus());
tasks.add(tmp);
System.out.println("Now you have " + tasks.getSize() + " tasks in the list.");

Choose a reason for hiding this comment

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

I appreciate how clean your code is in the try blocks! But maybe you could include more spacings and comments between different segments of the code so that it is easier to capture the main gist of your code?

}
catch (DukeException ex) {
System.out.println(ex.getMessage());
}
}
else if(cmd.length() >= 5 &&cmd.substring(0, 5).equals("event")) {
try {
checkCmd(cmd, 5, "☹ OOPS!!! The description of a event cannot be empty.");
String getName = parser.getNameAt(cmd);
String getTime = parser.getDeadlineAt(cmd);
getTime = formatDate(getTime);
Event tmp = new Event(getName, getTime);
System.out.println("Got it. I've added this task: ");
System.out.println(" " + tmp.getStatus());
tasks.add(tmp);
System.out.println("Now you have " + tasks.getSize() + " tasks in the list.");
//
}
catch (DukeException ex) {
System.out.println(ex.getMessage());
}
}
else if(cmd.length() >= 6 && cmd.substring(0, 6).equals("delete")){
int c = 0;
for(int i = 7; i < cmd.length(); ++i) {
c = c * 10 + cmd.charAt(i) - '0';
}
System.out.println("Noted. I've removed this task: ");
System.out.println(tasks.get(c - 1).getStatus());
tasks.remove(c - 1);
System.out.println("Now you have " + tasks.getSize() + " tasks in the list.");
}
else if(cmd.length() >= 4 && cmd.substring(0, 4).equals("find")) {
String tmp = cmd.substring(5);
System.out.println("Here are the matching tasks in your list:");
for(int i = 1; i <= tasks.getSize(); ++i) if(tasks.get(i - 1).description.contains(tmp)){
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Perhaps you could use Egyptian style brackets for the 'for' loop? Also, maybe you can consider writing the 'if' block on the next line below the 'for' statement? I think this can make the code look cleaner and enhances readability.

System.out.println(i + "." + tasks.get(i - 1).getStatus());
}
}
else System.out.println("☹ OOPS!!! I'm sorry, but I don't know what that means :-(");
Copy link

@jonasngs jonasngs Aug 31, 2020

Choose a reason for hiding this comment

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

Perhaps the conditional else statement body should be put on a separate line, i.e line 128? Also, maybe you could wrap the body of the else statement in curly braces to enhance readability?

storage.updateDataFile(tasks.getArrayList());
}
}

/**
* check if date is yyyy-mm-dd, then format to MMM d yyyy
*/
public static String formatDate(String str) {
LocalDate d;
try {
d = LocalDate.parse(str);
} catch (Exception e) {
return str;
}
return d.format(DateTimeFormatter.ofPattern("MMM d yyyy"));
}

/**
* check if the command is valid
*/
public static void checkCmd(String cmd, int len, String Ex) throws DukeException {
if(cmd.length() == len) throw new DukeException(Ex);
}

}
9 changes: 9 additions & 0 deletions src/main/java/DukeException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//package duke;
/**
* handle error when using duke
*/
public class DukeException extends Exception {
public DukeException(String message) {
super(message);
}
}
42 changes: 42 additions & 0 deletions src/main/java/Event.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//package duke;
/**
* Event <- Task
*/
public class Event extends Task {
String description;
String time;
boolean isDone;

/**
* Init Event
*/
public Event(String description, String time) {
super(description);
this.time = time;
}

/**
* Status in format [E][x] return book (at: 6 June)
*/
@Override
public String getStatus() {
return "[E]" + super.getStatus() + " (at: " + time + ")";
}


/**
* type of Task -> E for Event
*/
@Override
public String getType() {
return "E";
}

/**
* description to write to data file
*/
@Override
public String getDescription() {
return super.getDescription() + "|" + this.time;
}
}
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

62 changes: 62 additions & 0 deletions src/main/java/Parser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//package duke;
/**
* translate command -> name and time
*/
public class Parser {

Choose a reason for hiding this comment

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

I really like how your Parser class separates different functions in a very intuitive manner to readers. However, perhaps you could provide a more detailed naming to the functions? This is so that the functionality of your functions can be more easily inferred by its name.


public Parser() {

}

Choose a reason for hiding this comment

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

Perhaps you can leave this constructor out since Java creates a default constructor if no constructor was declared. However, I do not think this is a serious issue. I noticed a similar issue in the Storage class below as well.


/**
* get name
* by command
*/
public String getNameBy(String cmd) {
for (int i = 0; i < cmd.length(); ++i) {
if (cmd.charAt(i) == '/' && cmd.charAt(i + 1) == 'b' && cmd.charAt(i + 2) == 'y') {
return cmd.substring(9, i - 1);
}
}
return "";
}

/**
* get deadline
* by command
*/
public String getDeadlineBy(String cmd) {
for (int i = 0; i < cmd.length(); ++i) {
if (cmd.charAt(i) == '/' && cmd.charAt(i + 1) == 'b' && cmd.charAt(i + 2) == 'y') {
return cmd.substring(i + 4);
}
}
return "";
}

/**
* getname
* at command
*/
public String getNameAt(String cmd) {
for (int i = 0; i < cmd.length(); ++i) {
if (cmd.charAt(i) == '/' && cmd.charAt(i + 1) == 'a' && cmd.charAt(i + 2) == 't') {
return cmd.substring(6, i - 1);
}
}
return "";
}

/**
* get time
* at command
*/
public String getDeadlineAt(String cmd) {
for (int i = 0; i < cmd.length(); ++i) {
if (cmd.charAt(i) == '/' && cmd.charAt(i + 1) == 'a' && cmd.charAt(i + 2) == 't') {
return cmd.substring(i + 4);
}
}
return "";
}
}
Loading