-
Notifications
You must be signed in to change notification settings - Fork 433
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
[cleowenxuan] iP #360
base: master
Are you sure you want to change the base?
[cleowenxuan] iP #360
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
Merge Level8 with master.
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.
Generally, great job following the coding standards! However, I feel that it would be better to have whitespaces between certain characters such as operators and braces. I believe it would help in ensuring your code follows a coding standard by installing the checkstyle plugin on IntellJ! 😄
src/main/java/Asher.java
Outdated
try { | ||
File file = new File(filePath); | ||
if (!file.exists()) { | ||
boolean created = file.createNewFile(); |
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.
Should this be renamed with a prefix preceding the boolean variable created
?
src/main/java/Asher.java
Outdated
import java.util.Scanner; | ||
import java.util.List; | ||
import java.util.ArrayList; | ||
import java.time.LocalDate; | ||
import java.time.LocalTime; |
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.
Would it be better to have the import statements be in alphabetical order?
src/main/java/Event.java
Outdated
@Override | ||
public String writeToString() { | ||
String status = isDone ? "1" : "0"; | ||
return "E" + " | " + status + " | " + description + " | " + startDate + " | " + startTime + " | " + endDate + " | " + endTime; |
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.
Would this be better if it was split into two lines?
src/main/java/Asher.java
Outdated
if (taskNumber != -1) { | ||
tasks.get(taskNumber).markDone(); | ||
System.out.println("Nice! I've marked this task as done:"); | ||
System.out.println(" " + tasks.get(taskNumber)); |
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.
Should the extra space between " " and + be removed?
Merge branch A-MoreOOP with the master.
Merging add-gradle-support with master.
Merge Java Doc branch with the master.
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.
Codes are mostly conforms to the Code Quality: Naming standards Great job 👍
There are just a few minor issues and suggestions that could be improved.
* @param tasks The List of tasks. | ||
*/ | ||
|
||
public void getFileContents(String filePath, TaskList tasks) { |
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 to remove the String filePath
as class variable is used, not a parameter.
src/main/java/asher/Asher.java
Outdated
*/ | ||
public void run() { | ||
String dataFile = "./taskLists.txt"; | ||
storage.getFileContents(dataFile, tasks); |
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.
Perhaps consider usinggetFileTasks
as tasks are scanned from .txt
file.
} | ||
|
||
@Override | ||
public String writeToString() { |
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 renaming toFileStringFormat
to be more descriptive.
src/main/java/asher/Ui/Ui.java
Outdated
* Displays the number of tasks in the list message. | ||
* @param totalTasks The total number of tasks. | ||
*/ | ||
public void showNumberOfTaskInListMessage(int totalTasks) { |
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 described the function with its name
src/main/java/asher/Tasks/Event.java
Outdated
DateTimeFormatter formattingDate = DateTimeFormatter.ofPattern("MMM dd yyyy"); | ||
DateTimeFormatter formattingTime = DateTimeFormatter.ofPattern("hh:mm a"); |
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 named them. Perhaps try DateFormatted
and FormattedTime
?
Some codes do not follow the coding standard. Refactor the packages in the asher package to follow the package naming convention. Refactor the java files inside test.java.asher to follow the class naming convention. Rephrase method/class header comments to follow the format specified in the coding standard. Java Docs are being added to most methods and classes for clarity.
Merge the branch branch-A-Assertions with master.
Merge the branch branch-A-CodeQuality with master.
Asher 🤖
Allow Asher to help you remember the things you need to do! It is,
FASTSUPER FAST to useAll 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 is the
main
method: