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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SeanChinJunKai] iP #209

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

Conversation

SeanChinJunKai
Copy link

@SeanChinJunKai SeanChinJunKai commented Jan 25, 2023

Duke

Duke frees your mind of having to remember things you need to do. It's,

  • text-based
  • easy to learn
  • -FAST SUPER FAST to use

All you need to do is,

  1. download it from here.
  2. double-click it.
  3. add your tasks.
  4. let it manage your tasks for you 馃槈

And it is FREE!

Features:

  • Managing tasks
  • Managing deadlines (coming soon)
  • Reminders (coming soon)

If you Java programmer, you can use it to practice Java too. Here's the main method:

public class Main {
    public static void main(String[] args) {
        Application.launch(MainApp.class,args);
    }
}

Copy link

@JamesLiuZX JamesLiuZX left a comment

Choose a reason for hiding this comment

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

Overall great coding standards! Can tell that you are an experienced programmer so I don't have much to add apart from exceptions and packaging which are not really coding standards. Keep up the good work!

break;
}
default:
throw new DukeException("Invalid command entered!");

Choose a reason for hiding this comment

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

Good cases of exception handling! The exceptions can be more specific if time permits, but since this is a chatbot that focuses heavily on UI, there isn't really a need for too much exception handling either beyond printing error messages.

Comment on lines +16 to +17
private static final String DIR_PATH = System.getProperty("user.home") + "/ip/data";
private String filePath;
Copy link

@JamesLiuZX JamesLiuZX Feb 2, 2023

Choose a reason for hiding this comment

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

Good job on setting the variables as private! Also for using final appropriately.

Copy link

Choose a reason for hiding this comment

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

Good work on making the file paths private

* @return Task object.
* @throws DukeException if there is an error reading the file.
*/
public static Task readTaskString(String input) throws DukeException {

Choose a reason for hiding this comment

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

This method could be under Parser, I think even though it is under the context of the text file it is still a parser method.

* @return string to be displayed to user.
*/

public String getResponse(String input) {
Copy link

Choose a reason for hiding this comment

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

maybe shift the getResponse method to the command class, this could simplify code in duke.java. Enum list could be in the parser class as well

Comment on lines +16 to +17
private static final String DIR_PATH = System.getProperty("user.home") + "/ip/data";
private String filePath;
Copy link

Choose a reason for hiding this comment

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

Good work on making the file paths private

*
* @author Sean Chin Jun Kai
*/
public class Task {
Copy link

Choose a reason for hiding this comment

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

good job on making javdocs for your methods

* @return chosen Task.
* @throws DukeException if given ID does not exist in TaskList.
*/
public Task getTask(int taskNumber) throws DukeException {
Copy link

Choose a reason for hiding this comment

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

good use of dukeException to convert the default array out of bounds exception to a custom one

SeanChinJunKai and others added 16 commits February 7, 2023 20:32
Duke methods does not adhere to code quality standards.

Poor code quality can result in readers not being able to understand the code whilst making it more prone to bugs and errors.

Usage of blank lines in order to implement logical structuring of code in the getResponse method and the removal of redundant method calls in the same method.

By laying out the code such that it adheres to logical structure, this makes it very clear for readers to easily tell at a glance what we are doing. Furthermore, the removal of redundant code can reduce the complexity of code being read by readers allowing them to better understand our intentions as the developer.
Duke methods does not adhere to code quality standards.

Poor code quality can result in readers not being able to understand the code whilst making it more prone to bugs and errors.

Usage of blank lines in order to implement logical structuring of code in the getResponse method and the removal of redundant method calls in the same method.

By laying out the code such that it adheres to logical structure, this makes it very clear for readers to easily tell at a glance what we are doing. Furthermore, the removal of redundant code can reduce the complexity of code being read by readers allowing them to better understand our intentions as the developer.
Parser methods do not adhere to code quality standards.

Poor code quality can result in readers not being able to understand the code whilst making it more prone to bugs and errors.

Usage of blank lines in order to implement logical structuring of code whilst implementing guard clauses to make happy path prominent.

By laying out the code such that it adheres to logical structure, this makes it very clear for readers to easily tell at a glance what we are doing. Furthermore, the use of guard clauses prevent readers from getting distracted by alternative paths taken when error conditions happen
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

3 participants