-
Notifications
You must be signed in to change notification settings - Fork 429
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
[Boo Wen Jun] iP #454
base: master
Are you sure you want to change the base?
[Boo Wen Jun] iP #454
Conversation
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 only looked at Code Quality → Naming, this is from week 4, W4.6, take note of this to improve your code and use it in the future as well. Well Done!
public class marsException extends RuntimeException{ | ||
public marsException(String message){ |
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 class marsException extends RuntimeException{ | |
public marsException(String message){ | |
public class MarsException extends RuntimeException{ | |
public MarsException(String message){ |
I noticed that the class name marsException does not follow the PascalCase convention for class names. According to the coding standard, class names should be nouns and written in PascalCase. Consider renaming it to MarsException to align with the standard.
protected String description; | ||
protected boolean isDone; | ||
|
||
public Task(String description) { | ||
this.description = description; | ||
this.isDone = false; |
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 like how you used a boolean variable name like isDone, which follows the convention of starting with 'is' to indicate a boolean value. This makes the code more readable and intuitive. Good job!
|
||
public class Mars { | ||
public static void main(String[] args) { | ||
List<Task> arrayList = new ArrayList<>(); |
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.
List<Task> arrayList = new ArrayList<>(); | |
List<Task> taskList = new ArrayList<>(); |
The variable name arrayList could be improved to better describe its purpose. Since this is a collection of tasks, consider renaming it to tasks or taskList.
} | ||
|
||
/*Level 2. Add, List */ | ||
private void store(List<Task> arrayList){ |
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 store() is a bit ambiguous as it doesn't clearly explain what it does. Based on its functionality, a more descriptive name like addAndListTasks() or manageTasks() would better convey its purpose and improve code clarity
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.
Overall, I found your code fine and easy to understand. You can improve your codes by following the coding style and splitting codes into multiple classes to engage more OOP.
"____________________________________________________________\n" + | ||
" Bye. Hope to see you again soon!\n" + | ||
"____________________________________________________________"); */ | ||
try{ |
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.
try{ | |
try { |
A space should be inserted here.
} | ||
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.
} | |
else { | |
} else { |
Should be modified to follow the coding conventions.
String description = reader.nextLine(); | ||
System.out.println("____________________________________________________________\n"); | ||
switch(taskType){ | ||
case "todo": |
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.
There should not be any indent before "case" statements.
private void mark(List<Task> lst){ | ||
Object[] taskArray = lst.toArray(); | ||
|
||
Scanner reader = new Scanner(System.in); |
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.
Don't you think that the creation of a new Object[]
is redundant here?
My suggestion is to remove this line and get the task object directly from the given List<task>
as lst.get(num - 1)
.
No description provided.