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

[RyanQiu1] iP #75

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

[RyanQiu1] iP #75

wants to merge 54 commits into from

Conversation

RyanQiu1
Copy link

@RyanQiu1 RyanQiu1 commented Jan 21, 2023

Duke Bot

Bot that is here to help you and your tasks out!

Duke bot is:

  • Texted Based
  • Easy to use
  • Slow Fast!

Here are the steps involved!

  1. Download the Jar file from here
  2. Click It!
  3. Add your tasks!
  4. Use it daily! 😀

Most importantly Free Software

Features in this software

  • Add todo tasks, events and deadlines
  • Delete tasks whenever you want
  • Saves your tasks and updates automatically!

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);
    }
}

damithc and others added 23 commits July 31, 2022 17:20
…he user, first is to handle the unknown command error and next is to handle the empty field error
…changes. Load the data from the hard disk when Duke starts up.
# Conflicts:
#	src/main/java/duke/parser/Parser.java
#	src/main/java/duke/task/Task.java
#	src/main/java/duke/task/TaskList.java
# Conflicts:
#	src/main/java/duke/parser/Parser.java
@RyanQiu1 RyanQiu1 changed the title RyanQiu1 [RyanQiu1] iP Jan 28, 2023
Copy link

@joyngjr joyngjr left a comment

Choose a reason for hiding this comment

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

Looks good!
Most of your code follows the coding standards, just that some minor tweaks are needed for your boolean methods, switch-case indentation and if-else blocks.

this.listNum = taskNumberMark;
}

public boolean execute(TaskList tasks, Ui ui, StorageList storage) {
Copy link

Choose a reason for hiding this comment

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

Perhaps this boolean method can be made to sound like a boolean (eg. canExecute)?

this.to = checkerTo[1];
}

public boolean execute(TaskList tasks, Ui ui, StorageList storage) {
Copy link

Choose a reason for hiding this comment

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

I noticed your naming convention for boolean methods in several other places too. Might be good to make them sound more like a boolean.

String[] input2 = input.split(" ");
Type t = Type.valueOf(input2[0].toLowerCase());
switch (t) {
case todo:
Copy link

Choose a reason for hiding this comment

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

Should the indentation of your switch-case statement be the same?

public void updateStorage() {
try {
File dir = new File("data");
if (!dir.exists()) dir.mkdirs();
Copy link

Choose a reason for hiding this comment

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

Shouldn't your conditionals be placed on a different line, and be wrapped in curly braces?


public String toString() {
if (typeofTask.equals(""))
return "[" + this.getStatusIcon() + "]" + " " + this.description;
Copy link

Choose a reason for hiding this comment

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

Same issue as with StorageList.java, shouldn't the if-block be wrapped in curly braces?

Copy link
Author

@RyanQiu1 RyanQiu1 Feb 3, 2023

Choose a reason for hiding this comment

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

Yes u are right, i have rectified it.

public ArrayList<Task> find(String message) {
ArrayList<Task> arrStr = new ArrayList<>();
for (Task t: list) {
if (t.toString().contains(message)) {
Copy link

Choose a reason for hiding this comment

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

Good job adding curly braces here! 👍

# Conflicts:
#	build.gradle
#	src/main/java/duke/Duke.java
#	src/main/java/duke/command/ByeCommand.java
#	src/main/java/duke/command/DeadlineCommand.java
#	src/main/java/duke/command/DeleteCommand.java
#	src/main/java/duke/command/EventCommand.java
#	src/main/java/duke/command/FindCommand.java
#	src/main/java/duke/command/ListCommand.java
#	src/main/java/duke/command/MarkCommand.java
#	src/main/java/duke/command/TodoCommand.java
#	src/main/java/duke/command/UnmarkCommand.java
#	src/main/java/duke/task/TaskList.java
RyanQiu1 and others added 9 commits February 3, 2023 09:56
There is a addition of a title and appears more aesthetically pleasing.

Adding the images and improving the UI, with setTitle() and getIcons().add().

To enable the user to see the application they are accessing and the icon of the software.
Added assertions and fixed the removed unused UI
Improve code quality by changing names, changing oop structure.
…-CodeQuality

# Conflicts:
#	data/duke.txt
#	src/main/java/duke/command/DeadlineCommand.java
#	src/main/java/duke/command/FindCommand.java
#	src/main/java/duke/command/MarkCommand.java
#	src/main/java/duke/command/TodoCommand.java
#	src/main/java/duke/command/UnmarkCommand.java
Copy link

@wxxedu wxxedu left a comment

Choose a reason for hiding this comment

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

Overall, I think your code is really structured and follows the coding standard very well.

private String timing;

public DeadlineCommand(String fullCommand) {
String[] checker = fullCommand.split("/by ");
Copy link

Choose a reason for hiding this comment

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

May I know why this is called checker???

* Delete Command to remove the tasks users do not want, according to its number on the list.
*/
public class DeleteCommand extends Command {
private int listNum;
Copy link

Choose a reason for hiding this comment

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

Can I clarify what listNum is used for? Would it be better to call it index or something?

public EventCommand(String input) {
String[] checkerSlash = input.split("/");
String[] checkerEvent = checkerSlash[0].split("event ");
String[] checkerFrom = checkerSlash[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.

Would it be better if we rename it to fromChecker or something related?

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