-
Notifications
You must be signed in to change notification settings - Fork 590
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
[Kok Bo Chang] iP #635
Open
C5hives
wants to merge
91
commits into
nus-cs2103-AY2425S1:master
Choose a base branch
from
C5hives:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[Kok Bo Chang] iP #635
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In build.gradle, the dependencies on distZip and/or distTar causes the shadowJar task to generate a second JAR file for which the mainClass.set("seedu.duke.Duke") does not take effect. Hence, this additional JAR file cannot be run. For this product, there is no need to generate a second JAR file to begin with. Let's remove this dependency from the build.gradle to prevent the shadowJar task from generating the extra JAR file.
The original Task class is superseded by the ToDo, Deadline and Event classes. Lawrence.java is updated to handle the different task types accordingly.
The single test case emulates user input and verifies that the Lawrence class responds correctly when fed a sequence of inputs. Ideally, more test cases should be incorporated in the future.
Most user misinputs will be recognised by the program instead of crashing unexpectedly. The test has also been updated to include some faulty inputs.
The enum class is also flexible enough to handle the capitalisation and partial capitalisation of commands.
This change is to facilitate initialising tasks that may already be complete, which will be utilised when reading from the hard disk.
Also renamed io methods for clarity
This will keep parsing logic localised and can be called accordingly depending on the source of the input
The factory will call the relevant class to parse input strings depending on its source
Also allows bot to initialise existing tasks on startup
The previous branch used to add javafx gui was add-gui, which was incorrectly named and not picked up by grading scripts. The proper branch branch-Level-10 was created with minimal commits with the sole intent of getting merged back into the master branch. Let's merge branch-Level-10 back into master, but leave the Level-10 tag as is so that the grading scripts mark the level 10 task as complete.
The current javadoc description for the method does not fully explain what instance fields the query matches when the method checks for a match. Changing the javadoc to specify that the description of the task is used for matching better explains the purse of the method. Let's modify the javadoc to specify that string matching is done with the task description to improve the documentation for the function.
The current implementation attempts to match the given input with the appriopriate CommandType enum value. If the input does not match any enum values, an IllegalArgumentException is thrown. This behaviour uses an exception for control flow, which should be avoided. Adding a special type to signify invalid command types might be a better way to handle bad inputs. Let's - Add an INVALID type for CommandType - Use the INVALID type as the default return value of CommandType::fromString
The current implementation for getResponse uses stores information about the previous executed command as a workaround for javafx components to access information about the executed command. This behaviour contributes to the temporary field code smell, as additional null checks need to be made if public methods are not called in the right order. Extracting out such information and placing them in a record class might be better to move data related to commands. Lets - Create a record object Response to store the data need by JavaFX for rendering - Refactor command objects to return a string response in the execute method instead of storing it as an instance variable to be accessed later - Clean up other parts of the program that require said data to use the Response record instead. - Remove unneeded instance fields that clutter the classes mentioned
Modify javadoc for TaskList::findTasks method
Rephrase javadocs for TaskList::findTasks
Branch a assertions
Branch a code quality
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Lawrence™ Experience
Need assistance with keeping things in check? Lawrence is here to assist you with that. This bot is:
Sneak Peek:
Installation
Download the .jar file from here.
list
taskscountry for you!It's FREE! (until the next GST hike) 🤩
Features