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

[Tan Yan-Hao Joshua] iP #305

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

Conversation

joshua-tyh
Copy link

@joshua-tyh joshua-tyh commented Jan 26, 2023

Sunday 👋

“Checkout my better half, Saturday on the iOS App Store.” – Team Saturday Devs

Sunday is less accomplished than Saturday on the iOS App Store, in that it lacks image-to-text capabilities. Instead 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, just like Saturday on the iOS App Store! 🤩

Features:

  • Managing tasks (todos, deadlines & events)
  • Functional GUI
  • Reminders (coming soon)
  • Sleek GUI (coming not so soon)

damithc and others added 19 commits July 31, 2022 17:20
Reorganised and fixed the try-catch block for MARK and UNMARK states.
Implemented a DELETE state for delete commands to remove tasks from the list. The NumberFormatException and IndexOutOfBoundsException are handled also accordingly.
Error messages no longer have an emoticon to remedy the unmappable character (0xB9) for encoding US-ASCII error.
Input format of dates and times follows "dd/mm/yyyy hhmm". Write format of dates and times follows "E, MMM dd yyyy, h:mm aa".

The text-ui-test inputs and expected outputs have also been updated accordingly.
Resolve using "mine" for Sunday.java, State.java and Printer.java files.
Deadline and Event tasks are now save with their date attributes according to the read format of ""dd/mm/yyyy hhmm". To achieve this, the Task class has been made abstract with the save() abstract method for child classes to override with their own implementations of how it should be saved.

A minor change was also made to the DataHandler class to remove the wildcard import. Redundant imports were also removed from the State class and Sunday class.
Created the Parser and Ui classes. Renamed State.java to Command.java, Records.java to TaskList.java and DataHandler.java to Storage.java.
Packages created:
- collections
	- TaskList.java
- command
	- Command.java
- exceptions
	- SundayException.java
- task
	- Deadline.java
	- Event.java
	- Task.java
	- Todo.java
-utilities
	- Parser.java
	- Storage.java
	- Ui.java
Set up gradle to build and run Sunday. Gradle has also been set up to run checkstyleMain with the checkstyle.xml file.
Copy link

@RubyNguyen07 RubyNguyen07 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!

src/main/java/Sunday.java Outdated Show resolved Hide resolved
src/main/java/Sunday.java Outdated Show resolved Hide resolved
src/main/java/collections/TaskList.java Outdated Show resolved Hide resolved
src/main/java/command/Command.java Outdated Show resolved Hide resolved
src/main/java/task/Deadline.java Outdated Show resolved Hide resolved
JUnit Tests for all Task types test the creation and saving of each task type.

Detected a bug when saving Deadline and Event tasks where the saved date and time was incorrect. The bug was fixed by changing the DataFomat from "dd/mm/yyyy hhmm" to "dd/MM/yyyy HHmm".
Fixed bug where save file was unable to be retrieved due to inaccurate relative file path. Text-Ui-Test files have also been updated.
Added a getFilepath method in Storage to accurately retrieve the save file directory.
Give users a way to find a task by searching for a keyword.
# Conflicts:
#	src/main/java/Sunday.java
#	src/main/java/collections/TaskList.java
#	src/main/java/command/Command.java
#	src/main/java/exceptions/SundayException.java
#	src/main/java/task/Deadline.java
#	src/main/java/task/Event.java
#	src/main/java/task/Task.java
#	src/main/java/task/ToDo.java
#	src/main/java/utilities/Parser.java
#	src/main/java/utilities/Storage.java
#	src/main/java/utilities/Ui.java
# Conflicts:
#	src/main/java/collections/TaskList.java
#	src/main/java/command/Command.java
#	src/main/java/task/Task.java
Used Gradle to check for checkstyle violations and amended them accordingly.
Update Sunday behaviour to take user inputs from GUI instead of the console.
Copy link

@ChangGittyHub ChangGittyHub 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 some nits to fix

src/main/java/utilities/Ui.java Outdated Show resolved Hide resolved
src/main/java/task/Task.java Show resolved Hide resolved
for (int i = 0; i < this.list.size(); i++) {
Task task = this.list.get(i);
if (task.match(keyword)) {
found.add(task);

Choose a reason for hiding this comment

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

Do you think this might be bit confusing? Maybe Tasklist instance can be called matchedList or foundList? the result could look like this: matchedList.add(task) which might be clearer

Printer.printBar();
public void execute(String input) throws SundayException {
boolean didSave = list.save();
Ui.printGoodbye(didSave);

Choose a reason for hiding this comment

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

maybe isSaved might be better?

//strArr[1] = "[" or "[X"
//strArr[2] = " \(description)"

switch (strArr[0].charAt(1)) {

Choose a reason for hiding this comment

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

could this be abstracted out?

//strArr[2] = " \(description)"

switch (strArr[0].charAt(1)) {
case 'T':

Choose a reason for hiding this comment

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

I like how you remember the right indentation for switch case

sb.append(strArr[j]);
sb.append(" ");
j++;
}
String description = sb.toString().substring(0, sb.length() - 1);
sb.setLength(0);
while (!strArr[j].equals("/to")) {
j++; // skip "/from" or "(from:"
while (!(strArr[j].equals("/to") || strArr[j].equals("to:"))) {

Choose a reason for hiding this comment

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

strArr[j] could be abstracted out for better readability?

@@ -1,10 +1,15 @@
public class Printer {
private final String bar = "____________________________________________________________";

Choose a reason for hiding this comment

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

could this be better if it was a final String named BAR instead of the change you made?

joshua-tyh and others added 20 commits February 16, 2023 21:04
Used Gradle to detect checkstyle violations and fixed them accordingly. Changes are made in a A-CheckStyle branch.
Use the JavaFX technology to implement the GUI. Commit changes to the Level-10 branch
# Conflicts:
#	src/main/java/utilities/Ui.java
Fixed the build.gradle file for the task shadowJar to create a JAR file.
Assert statements were added to document important assumptions that
should hold at various points in the code.

Let's add assert statements in the following,
* In the DEADLINE command to check that the deadline description
contains a "/by" or "(by:" field.
* In the EVENT command to check that the event description contains a
"/from" or "(from:" field.
* In the EVENT command to check that the event description contains a
"/to" or "to:" field.
* In the TODO command to check that the todo description is not empty.
* In the add(Task task) method to check that the task parameter is not
null.
* In the mark(int index) method to check that the index parameter is
within the bounds of the list.
* In the unmark(int index) method to check that the index parameter is
within the bounds of the list.
* In the delete(int index) method to check that the index parameter is
within the bounds of the list.
In the getTask	(int index) method to check that the index parameter is
within the bounds of the list.
A few methods throughout the codebase were not being called/utilised.

Removing all those redundant methods improves the code quality.

Let's remove those redundant methods and cleanup the codebase to improve
the code quality.
# Conflicts:
#	src/main/java/command/Command.java
To give proper ordering to the list of tasks, a sort method should be
added to allow the user to sort the tasks in the list.

Sorting the tasks in alphabetical order according to the task description
will enable the user to better locate tasks in the list.

To implement this feature, let's:
* Implement the Comparable interface in the Task class and override the
compareTo() method to compare task descriptions.
* Add a "Sort" command for the user to invoke the sort feature.
Updated codebase to comply with checkstyle requirements according to
gradle checkstyleMain.
Add a representative screenshot of the product to the docs folder named
"Ui.png".
Emojis were not showing up on the GitHub Pages view. Removing them to
improve readability.
Add javadoc for getResponse() in Sunday.java
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