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

[Wong Jun Long] iP #100

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

Conversation

junlong4321
Copy link

No description provided.

damithc and others added 29 commits July 23, 2020 23:27
Delete and gitignore class files
Delete and gitignore class files
# Conflicts:
#	text-ui-test/runtest.bat

Resolved by combining both FileManipulator and DateTime java compilation paths into runtest.bat
Conflicts:
	src/main/java/exception/DukeException.java
	src/main/java/parser/Parser.java
	src/main/java/storage/Storage.java
	src/main/java/task/TaskList.java
	src/main/java/task/tasks/Task.java

Conflicts occurred because of minor white spaces issues.
Amended code and integrated changes from both branches.
@EkChinHui
Copy link

Commonly used variables could be saved as a constant such as dividers in the TaskDescription class. Method names should be verbs describing an action and not be in the past tense such as the addedTaskDescription method. More whitespace could be added for bigger chunks of code to improve readability. Besides these, coding standards are abided by, with sufficient comments that aid in understanding the code base. Good job!

Copy link

@xz0127 xz0127 left a comment

Choose a reason for hiding this comment

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

The Javadoc comments are really detailed and clear. They makes the code base really easy to understand. There are several method names which could possibly be improved in terms of naming style and the spacing between the parameters name and descriptions can be more consistent. Overall, the coding standards are strictly abided by and there are very few violations. I really like the work done! :) 👍

/**
* Renders an error message that user input has been written incorrectly.
*/
public static String invalidCommand() {
Copy link

Choose a reason for hiding this comment

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

Perhaps use verbs for this and the following methods' names?

src/main/java/datetimeconverter/DateTimeConverter.java Outdated Show resolved Hide resolved
src/main/java/task/TaskDescription.java Show resolved Hide resolved
src/main/java/task/TaskList.java Outdated Show resolved Hide resolved
@zhaolingshan
Copy link

Your code is very readable and easy to understand with comprehensive java docs and additional detailed comments!
Your code mostly follows the java coding standard with very few exceptions such as the method names can be phrased in a better way but they are very minor. Good job!

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.

None yet

4 participants