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

[Qiu Qianhui] iP #376

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

[Qiu Qianhui] iP #376

wants to merge 60 commits into from

Conversation

QQH0828
Copy link

@QQH0828 QQH0828 commented Jan 31, 2023

Duke

“Your mind is for having ideas, not holding them.” – David Allen (Source)

Duke is a chatbot that frees your mind from having to remember the 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
  • Reminders
  • GUI interface

If you are a random programmer that's looking through this, you can use it to practice Java as well. Here is the main code for the launcher.

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

Copy link

@ShanHng ShanHng left a comment

Choose a reason for hiding this comment

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

Code is clean and easy to read! But there is repeated violation of the rule to have the [ ] placed behind the data type instead of variable name. Generally looks great!

throw new DukeException(("☹ OOPS!!! The due date of a deadline cannot be empty.\n"));
}

String info[] = command.split(" /by ");
Copy link

Choose a reason for hiding this comment

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

Perhaps we should put the [] right after the String instead even though this is valid too?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing them out!

throw new DukeException(("☹ OOPS!!! The due date of a deadline cannot be empty.\n"));
}

String info[] = command.split(" /by ");
String description = info[0];
String by = info[1];
Copy link

Choose a reason for hiding this comment

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

Maybe instead of just "by", we can use a variable name that indicates that this is a date?

throw new DukeException("\t ☹ OOPS!!! The description, from date or due date of a event cannot be empty.\n");
}

String info[] = command.split(" /from ");
Copy link

Choose a reason for hiding this comment

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

Perhaps the naming of this variable can be a standard plural noun instead?

}

public void event(String command) {
String info[] = command.split(" ", 2)[1].split(" /from ");
Copy link

Choose a reason for hiding this comment

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

Ooh I like your clever usage of split to distinguish between /to argument and /from argument!

public void addTask(Task task) {
tasks[taskIndex] = task;
taskIndex++;
System.out.println("\t Got it. I've added this task:\n"
Copy link

Choose a reason for hiding this comment

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

I like what you did here - indenting and spacing a sentence into parts that are more readable!

}

public void deadline(String command) {
String info[] = command.split(" ", 2)[1].split(" /by ");
Copy link

Choose a reason for hiding this comment

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

Perhaps we could use a variable name that indicates this is an array? Also, remember to have the square brackets right after the data type~! (String[])

# Conflicts:
#	src/main/java/Event.java
#             src/main/java/Deadline.java
# Conflicts:
#	src/main/java/Deadline.java
#	src/main/java/Event.java
#	src/main/java/Task.java
# Conflicts:
#	src/main/java/command/AddDeadlineCommand.java
#	src/main/java/command/AddEventCommand.java
#	src/main/java/command/AddTaskCommand.java
#	src/main/java/command/AddTodoCommand.java
#	src/main/java/command/Command.java
#	src/main/java/command/DeleteTaskCommand.java
#	src/main/java/command/ExitCommand.java
#	src/main/java/command/ListCommand.java
#	src/main/java/command/MarkTaskCommand.java
#	src/main/java/storage/Storage.java
#	src/main/java/userinteraction/Ui.java
QQH0828 and others added 30 commits February 16, 2023 01:16
Assertions are used to define assumptions about the program state
so that the runtime can verify them.

Adding assertions in the code statements so that it can check whether
the split operations used in the input string result in the correct
output.

It can help to detect the mistakes in the codes.
Some of the classes had some code quality violation that needed to
be fixed according to the code quality rules.

Deleting dead code and unused codes so that not trip up readers.

Avoiding deep nesting and complicated expressions so that much more
easier for reading.

Trying to follow the single level of abstraction principle.

These are helping to run-time efficiency and understandability.
Add assert statement in the TaskList class
* 'master' of https://github.com/QQH0828/ip:
  Add assert statement in the TaskList class
Modify the codes based on the code quality
* 'master' of https://github.com/QQH0828/ip:
  Modify the codes based on the code quality
Adding help command in the Duke to display all available commands
with the explanation about how to use each commands.
Add a new line in each classes.
Remove whitespace of "trailing whitespace".
Add a space before "{".
Add and change some java doc.
Mainly from gui part.
Solve the issues about spacing after some commands.
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.

2 participants