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

[ZiliaAJY] iP #26

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

[ZiliaAJY] iP #26

wants to merge 43 commits into from

Conversation

ZiliaAJY
Copy link

No description provided.

- Implement Task class for managing tasks
- Add Conglo$Task.class file and Conglo.class file
- Refined method and variable names for consistency and clarity
- Adjusted layout and spacing for improved readability
- Add 'Task.java', 'Deadline.java', 'Todo.java' and 'Event.java' files
- Implement classes Todo, Deadline and Event classes to inherit from a Task class
- Refactored method names and constants for improved clarity
- Added JavaDoc comments to Task, Deadline, Event, and Todo classes
  - Included class-level documentation and descriptions for constructors and methods.
  - Documented parameters, return values, and method behaviors
Copy link

@Chiang-HuiXin Chiang-HuiXin left a comment

Choose a reason for hiding this comment

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

Good job! Overall, I think that your code is very neat and readable. There are good abstractions and documentation.

Comment on lines 145 to 147
Todo todo = new Todo(words[1]);
taskList[taskCount] = todo;
taskCount++;

Choose a reason for hiding this comment

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

Maybe put this in a method?

*
* @param args Command-line arguments (not used).
*/
public static void main(String[] args) {

Choose a reason for hiding this comment

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

I like how your main method is short!

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

public void markAsDone() {

Choose a reason for hiding this comment

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

Maybe name it as "setMarkAsDone" since it is a setter?

Comment on lines 19 to 25
/**
* Returns a string representation of the Deadline task.
* The format includes a label for deadlines, the task's completion status,
* description, and the deadline date.
*
* @return A string representation of the Deadline task.
*/

Choose a reason for hiding this comment

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

I like that you do documentation, it really makes the code much more readable!

Comment on lines 37 to 39
/**
* Prints a line separator to the console.
*/
Copy link

Choose a reason for hiding this comment

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

I like how you have added comments that describe what each function does.

Comment on lines 24 to 35
public static void main(String[] args) {
greetUser();
Scanner scanner = new Scanner(System.in);
Task[] listing = new Task[MAX_TASKS];
command = scanner.nextLine();
while (!command.equals("bye")) {
processCommand(command, listing);
command = scanner.nextLine();
}
scanner.close();
sayGoodbye();
}
Copy link

Choose a reason for hiding this comment

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

I like how your main function is readable and easy to follow!

*/
public static void processCommand(String command, Task[] taskList) {
String[] words = command.split(" ", 2);
switch(words[0]) {
Copy link

Choose a reason for hiding this comment

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

Your switch case indentation looks good, great job!

echoTask(todo);
break;
case "deadline":
addDeadline(words[1], taskList);
Copy link

Choose a reason for hiding this comment

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

I like that your functions are named concisely and easy to understand.

- Renamed paths in runtest.bat
- Added new tasks and commands to test various functionalities, including:
  - Adding and listing tasks
  - Marking tasks as done
  - Handling multiple commands in sequence
-Contains Conglo's custom exception including unknown command, missing description, invalid or missing task number and invalid format
- Use base CongloException for common exception handling
- Added internal try-catch block to handle specific exceptions
- Improved error handling and message output
- Created packages "conglo", "conglo,exception", "conglo.manual" and "conglo.task"
- Save tasks automatically to the hard disk whenever the task list is modified.
- Load tasks from the hard disk when the application starts, ensuring persistence across sessions.
- Hard-coded file path and name for storage: './data/conglo.txt'.
- Added error handling for file I/O operations to manage file creation and loading processes.
- Replaced all instances of normal arrays with ArrayLists for better flexibility.
- Implemented functionality to delete tasks from the list.
Add task deletion functionality

- Implement support for deleting tasks from the list.
- Update task management to handle deletions correctly.
- Ensure task list updates reflect changes immediately.
- Resolved conflicts in Task.java
- Resolved conflicts in Conglo.java

Description of changes made to resolve conflicts:
- Fixed discrepancies in file paths and format between branches
- Refactor Task class and its children class to adopt new format for saving of tasks
- Save tasks automatically to the hard disk whenever the task list is modified.
- Load tasks from the hard disk when the application starts, ensuring persistence across sessions.
- Hard-coded file path and name for storage: './data/conglo.txt'.
- Added error handling for file I/O operations to manage file creation and loading processes.
Merge branch-Level-7 to include Save function
Copy link

@okkhoy okkhoy left a comment

Choose a reason for hiding this comment

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

There are a few coding standards and code quality violations in the code. Please try to fix them before the final submission.

@@ -0,0 +1,253 @@
package conglo;

import conglo.manual.*;
Copy link

Choose a reason for hiding this comment

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

don't use wildcard imports

*/
public class Conglo {

// Constants
Copy link

Choose a reason for hiding this comment

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

redundant comment

} else {
System.out.println("All done! Task added to list:");
}
String taskSuffix = size > 1 ? " tasks" : " task";
Copy link

Choose a reason for hiding this comment

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

could consider making this "easy to read" by using explicit if ... else block.

* @throws CongloException.InvalidFormat If the deadline format is incorrect.
*/
public static void addDeadline(String sentence) throws CongloException.InvalidFormat {
if (!sentence.contains(" /by ")) {
Copy link

Choose a reason for hiding this comment

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

can try improving SLAP in such cases

}

public static void deleteTask(String word) throws CongloException.InvalidTaskNumber {
int i;
Copy link

Choose a reason for hiding this comment

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

use a more descriptive variable name.

}
listTasks();
break;
case "unmark":
Copy link

Choose a reason for hiding this comment

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

fall through comment is needed

}

// Custom exception for unknown commands
public static class UnknownCommand extends CongloException {
Copy link

Choose a reason for hiding this comment

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

consider making these separate classes rather than nested classes

private static final String FILE_PATH = "./data/conglo.txt";
private static final String DIRECTORY_PATH = "./data";

public static ArrayList<Task> loadTasks() {
Copy link

Choose a reason for hiding this comment

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

you can improve SLAP here

case "D":
if (parts.length < 4) {
System.out.println("Invalid deadline format: " + line);
continue;
Copy link

Choose a reason for hiding this comment

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

is there a way to remove arrow head code here?

- Created the Ui class for user interface interactions and message displays
- Implemented the Storage class for task storage and file management
- Introduced the Parser class to handle command parsing and user input processing
- Created the TaskList class to enhance task management functionalities
- Improved code organization and readability
- Removed use of ArrayList<> in Parser.java
- Cleaned up code for readability
- Added DateParser class to handle date and time input for deadlines
- Updated Deadline class to store deadline dates as LocalDate and format output to "dd MMM yyyy"
- Enhanced user input acceptance for dates in dd-MM-yyyy HHmm format
- Implemented conversion of 24-hour time format to am/pm in deadline outputs
- Update Manual.java to include the newly accepted format
- Added a method to search for tasks by keyword within the TaskList class
- Created a findTasks() method that returns a list of matching tasks
- Updated the Parser class to handle user commands for finding tasks
- Updated Manual.java to include instruction for finding tasks
…umentation

- Implemented header comments for all public and non-private classes.
- Added Javadoc comments to all non-trivial methods, enhancing clarity.
Add Javadoc comments to all classes and methods for comprehensive doc…
@ZiliaAJY ZiliaAJY force-pushed the master branch 5 times, most recently from 3cf1ea8 to 19d3c30 Compare October 1, 2024 16:19
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