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

[Hue Koh] iP #456

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

[Hue Koh] iP #456

wants to merge 76 commits into from

Conversation

huekoh
Copy link

@huekoh huekoh commented Feb 5, 2024

GMO (GameMate Organizer) 👾 🚀

"Who wants to play video games?" - GMO

GMO's not a bad influence... he

  • helps you remember your tasks
  • SULKS reminds you when you are slaved by work
  • tells you to hurry so you can play with him!

All you need to do is

  1. Download GMO here
  2. Double click it
  3. Start telling GMO your dues
  4. Let him keep track of them for you 🤩

And he is FREE

GMO's Features:

  • Keep track of tasks (todos, dues, events)
  • Manage task status
  • Sort tasks by date
  • Tell you to skip lecture

And if you are a Java programmer, use him to practice your coding too! Here's GMO's constructor method:

public GMO() {
        ui = new Ui();
        storage = new Storage();

        try {
            tasks = new TaskList(storage.loadData(), ui, storage);
        } catch (IOException e) {
            System.out.println("Error: Unable to load data. " + e.getMessage());
            tasks = new TaskList(ui);
        }
    }

damithc and others added 13 commits January 7, 2024 18:33
Let's tweak the docs/README.md (which is used as the user guide)
to fit Duke better. Specifically,

1. mention product name in the title
2. mention adding a product screenshot and a product intro
3. tweak the flow to describe feature-by-feature
BMO now echoes back everything you say to him
abstracted tasks into a separate class
Error handling now handles these:
1. general error
2. index provided is out of range
3. task to be done is already done
4. task to be redone is already not done
5. no index was provided
6. user input add with nothing following up
7. incorrect deadline input format
8. incorrect todo input format
9. incorrect event input format
@huekoh huekoh changed the title [Hue Koh] i [Hue Koh] iP Feb 5, 2024
The formatter now recognises date time strings with "/" to express dates as valid. Previously, strings containing "/" would throw a format error and not accept it as a valid string
When user adds a task with a date time variable, the string will be formatted into a LocalDateTime object that is stored in the task object.
Copy link

@nichee nichee left a comment

Choose a reason for hiding this comment

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

All good! Just small naming issues here and there

static void viewLog() {
StringBuilder logPrint = new StringBuilder();
if (taskLog.isEmpty()) {
System.out.println(Constants.emptyLogPrint);
Copy link

Choose a reason for hiding this comment

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

Consider renaming this function to "printEmptyLog" to start the name with the verb

int index = Integer.parseInt((input));

if (index > taskLog.size() || index <= 0) {
System.out.println(Constants.errorPrint.outOfRange());
Copy link

Choose a reason for hiding this comment

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

Consider changing the naming convention of the printing functions like outOfRange, perhaps use a verb?

@@ -0,0 +1,11 @@
public class ToDos extends Task {
Copy link

Choose a reason for hiding this comment

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

As far as I can tell this is the Todo class and not multiple Todos? Perhaps this can be changed to Todo since its used to instantiate single instances, so "ToDos" so it may be confusing

Copy link

@qinboan qinboan left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits to fix!

Comment on lines 5 to 6
import java.util.*;
import java.lang.*;
Copy link

Choose a reason for hiding this comment

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

Avoid using wildcard imports as per java coding standard!

String taskDescription = info[2].trim();

switch (taskType) {
case "T":
Copy link

Choose a reason for hiding this comment

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

There should be no indentation for case clauses

return (isDone ? "[X]" : "[ ]"); // mark done task with X
}

public Boolean getStatus() {
Copy link

Choose a reason for hiding this comment

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

Consider renaming this boolean method to sound like booleans.

Suggested change
public Boolean getStatus() {
public Boolean isDone() {

return taskLog;
}

// Other methods for task management...
Copy link

Choose a reason for hiding this comment

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

Maybe you should delete this comment as it serves no purpose :)

Copy link

@HusseinSafwan02 HusseinSafwan02 left a comment

Choose a reason for hiding this comment

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

Just some minor fixes, well done!

+ " Let's play mario kart right now!!\n"
+ "----------------------------------------------------------------------------------\n";

public static class errorPrint {

Choose a reason for hiding this comment

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

maybe you can consider renaming the class to errorPrints, since it has multiple error messages

}

private boolean isValidContentFormat(String content) {
// Implement your logic to check whether the content format is valid

Choose a reason for hiding this comment

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

I'm not very sure of the purpose of the comment here


static void parseData(String content) {
String[] lines = content.split("\n");
Integer indexCounter = 0;

Choose a reason for hiding this comment

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

you can use int here else there is unnecessary unboxing

huekoh and others added 30 commits February 19, 2024 00:20
Add assertions to the BMO, Storage and TaskList classes

Several methods in these classes require important assumptions to hold true in order for the program to run.

Assertions for these conditions to the code helps the debugging process.

Let's add assertions to these classes.
Parse parse method is too long and handles both input type checking and input format checking.

Having smaller methods to handle the different types of possible inputs makes the code more readable.

Let's update the parser method to handle only input type checking, and abstract out input format checking to appropriate methods.
Add assertions to Deadlines and Events classes
Parse parse method is too long and handles both input type checking and input format checking.

Having smaller methods to handle the different types of possible inputs makes the code more readable.

Let's update the parser method to handle only input type checking, and abstract out input format checking to appropriate methods.
TaskList constructor method is too long and has duplicated code.

Swapping around the try block to encapsulate the switch block instead of vice versa helps remove duplicate code and improve readability.

Let's update the TaskList constructor method and reduce duplicate codes
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.

5 participants