-
Notifications
You must be signed in to change notification settings - Fork 434
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
[Rena] iP #458
base: master
Are you sure you want to change the base?
[Rena] iP #458
Conversation
Let's tweak the docs/README.md (which is used as the user guide) to fit Duke better. Specifically, 1. mention product name in the title 2. mention adding a product screenshot and a product intro 3. tweak the flow to describe feature-by-feature
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.
LGTM. Small nits only
src/main/java/Duke.java
Outdated
current.unmark(); | ||
System.out.println(printMark(current)); | ||
}else{ |
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.
I think you may have forgotten to add a space before and after the 'else' here.
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.
Just some nits that require attention, mainly related to coding standards and naming.
src/main/java/Duke.java
Outdated
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.
Could probably store the lines as a constant value to make the code easier to change.
src/main/java/Duke.java
Outdated
"____________________________________________________________\n"; | ||
} | ||
|
||
public static ArrayList<Task> delete(ArrayList<Task> a, int count) { |
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.
The method name could be changed to explain more about what this method is deleting.
src/main/java/Duke.java
Outdated
Scanner input = new Scanner(System.in); | ||
System.out.println("Enter Message"); | ||
|
||
final int ArraySize = 100; |
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.
Constants should be in screaming_snake_case.
src/main/java/Duke.java
Outdated
String task = getTask(message); | ||
String[] parts = time.split("by"); | ||
// tasks[counter] = new Deadline(task, parts[1]); | ||
tasks.add(new Deadline(task, parts[1]) ); |
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.
tasks.add(new Deadline(task, parts[1]) ); | |
tasks.add(new Deadline(task, parts[1])); |
src/main/java/Duke.java
Outdated
} | ||
} | ||
|
||
public static String added(Task task, int count) { |
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.
public static String added(Task task, int count) { | |
public static String printAdded(Task task, int count) { |
Methods should have verbs as names.
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.
Some syntax issues to fix, as well as naming conventions, but well done so far!
src/main/java/DataManager.java
Outdated
String status = " "; | ||
if(task instanceof Todo) { | ||
symbol = "T"; | ||
}else if(task instanceof Deadline) { |
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.
could insert a space between } and else if
src/main/java/Duke.java
Outdated
if (message.startsWith("todo")) { | ||
String[] parts = message.split(" ", 2); | ||
String task = parts[1]; | ||
// tasks[counter] = new Todo(task); |
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.
consider removing these comments once code is finalised :)
# Conflicts:conflict in Duke.java # src/main/java/Duke/Duke.java
# Conflicts: Conflicts between level-9 and master branches # src/main/java/Duke/Duke.java # src/main/java/actions/Parser.java # src/main/java/actions/TaskList.java
Introduced assertions in the Parser class for handle methods to validate assumptions about method inputs. This enhances debugging and code reliability during development.
Arrow head code is reduced These changes enhance maintainability and readability of the code
Add critical assertions across to the parser class
CodeQuality
Duke
Duke helps you record your tasks and makes it easier for you to manage them.
All you need to do is,
And it is FREE!
Features:
If you are a Java programmer, you can use it to practice Java too. Here's tje main method: