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

[janelleenqi] iP #75

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

Conversation

janelleenqi
Copy link

pull request 1

@janelleenqi janelleenqi closed this Sep 5, 2023
@janelleenqi janelleenqi reopened this Sep 5, 2023
@janelleenqi janelleenqi changed the title janelleenqi iP [janelleenqi] iP Sep 5, 2023
System.out.println("Hello! I'm\n" + logo);
System.out.println("What can I do for you?");

Task[] theList = new Task[100];

Choose a reason for hiding this comment

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

Consider using a more descriptive name such as taskList instead of theList


String echo = userInput.nextLine();

while (!echo.equals("bye")) {

Choose a reason for hiding this comment

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

Consider declaring static final constants for each command (e.g. bye, list, mark) so that they can be easily referenced and updated for future use

Suggested change
while (!echo.equals("bye")) {
private static final String BYE_COMMAND = "bye";
while (!echo.equals("bye")) {

@@ -0,0 +1,27 @@
public class Task {

Choose a reason for hiding this comment

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

Good job for using good coding standards in this file! Descriptive names are used for functions and variables

Scanner userInput = new Scanner(System.in);
int counter = 0;

String echo = userInput.nextLine();

Choose a reason for hiding this comment

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

Can consider making some changes to the following variables:

  • echo should be renamed to userInputString instead since your default command is now adding a new task
  • userInput should contain the scanner keyword such as inputScanner to better signify that it is the scanner object

Copy link

@SebasFok SebasFok left a comment

Choose a reason for hiding this comment

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

Clear code and good use of white space to make the code clean and easy to read. Good job!

while (!echo.equals("bye")) {
String[] words = echo.split(" "); //to identify usage of features "mark" & "unmark"

if (echo.equals("list")) {
Copy link

Choose a reason for hiding this comment

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

Could potentially use a switch if possible to make the code more readable.


Task[] theList = new Task[100];
Scanner userInput = new Scanner(System.in);
int counter = 0;
Copy link

Choose a reason for hiding this comment

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

Perhaps the counter variable could be more descriptive to allow the reader to understand what it is counting.

}

//...
}
Copy link

Choose a reason for hiding this comment

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

I like your use of descriptive names to make the code clear and concise.

Copy link

@YongbinWang YongbinWang left a comment

Choose a reason for hiding this comment

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

Overall good job for your ip so far. Take note of how you name your variables, using clear and appropriate names can improve the readability of your code. Keep up the good work!

Scanner userInput = new Scanner(System.in);
int counter = 0;

String echo = userInput.nextLine();

Choose a reason for hiding this comment

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

Consider using a more suitable name for echo so that it is more descriptive for the reader.

Comment on lines 27 to 28
switch (action) {
case LIST:

Choose a reason for hiding this comment

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

There is a violation of Java coding standard here. There should be no indentation for case clauses. Please configure your IDE to follow the coding standard.


case DEADLINE:
taskDescription = echo.substring(8);
slashCut = taskDescription.indexOf("/");

Choose a reason for hiding this comment

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

Consider renaming this variable to make it more descriptive.

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.

4 participants