[RobotWizzard] iP #673
Open
[RobotWizzard] iP #673
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.
…s when running the 'mark' and 'unmark' commands
…e when parsing commands with arguments; fix incorrect throwing of exception for the 'mark' and 'unmark' commands
LowXiSi
reviewed
Sep 8, 2024
src/main/java/Bob.java
Outdated
| System.out.println(logo); | ||
| greet(); | ||
| while (true) { | ||
| boolean executed = false; |
There was a problem hiding this comment.
Should executed be named as isExecuted instead to show that it is a boolean value?
src/main/java/Bob.java
Outdated
| private static final String LINE_PREFIX = " "; | ||
|
|
||
| private static String argument = ""; | ||
| private static final List<Task> list = new ArrayList<>(); |
There was a problem hiding this comment.
Should list be named as tasks instead to represent a collection of tasks?
potaotototo
reviewed
Sep 8, 2024
src/main/java/Bob.java
Outdated
| StringBuilder text = new StringBuilder(); | ||
| int i; | ||
| for (i = 0; i < list.size() - 1; ++i) { | ||
| text.append(i + 1).append(".").append(list.get(i)).append("\n"); |
There was a problem hiding this comment.
Is there anyway that this could be abstracted? For example, maybe a method inside the Task class or its inheriting classes?
src/main/java/Bob.java
Outdated
Comment on lines
80
to
82
| String desc = fromIndex < toIndex | ||
| ? argument.substring(0, fromIndex) | ||
| : argument.substring(0, toIndex); |
There was a problem hiding this comment.
Not sure whether this is accurate? Maybe try to add comments to explain why you choose to incorporate fromIndex and toIndex?
src/main/java/Bob.java
Outdated
| } | ||
| int i; | ||
| try { | ||
| i = Integer.parseInt(argument) - 1; |
There was a problem hiding this comment.
Not compulsory but could try to name i as something else. This can help other programmers understand the code easily.
src/main/java/Bob.java
Outdated
| } | ||
| }; | ||
|
|
||
| public final String CMD; |
There was a problem hiding this comment.
I think the coding standards mentioned that we should avoid short forms? Maybe can name as COMMAND instead of CMD?
RobotWizzard
force-pushed
the
master
branch
from
a920d7c
to
fa5b5aa
Compare
Add assertions throughout the code
The project repository is getting messy. Classifying classes into packages improves the code quality. Let's package all the utility classes into bob.util. Utility classes are defined as classes with only static methods.
The repository is getting a bit messy. Let's extract the utility classes into their own package. Utility classes are classes that only have static methods.
There are ways for users to tag tasks, but no way for them to remove tags. The "untag" command is added for users to untag specific tags from a task, or all tags from a task.
There is a need for classes to get the unformatted version of a string. A FormattedString class would also enable greater flexibility when formatting strings in the future, for example applying many formats to a string. Let's extract a FormattedString class that can apply red and yellow-background to strings for now. The FormattedString class acts like the StringBuilder class in Java which allows chaining operations on it.
There is currently no way to combine separate FormattedString objects. Let's add an append method to FormattedString to allow appending other FormattedString objects to it.
The app currently has the default icon and title, which shows at the top of the window. Adding a title and icon would improve the user experience. Let's add a title and icon to our app.
The current background image does not work well as it stretches and cannot be repeated, which does not look good. Having a good background would improve the user experience. Let's change the background to a repeating one.
The app currently have square profile picture, which is different from what users may want. Let's make the profile pictures circular to match the look of a chat app.
The current dialog boxes have a background color that stands out too much, and a border with multiple colors, which is too distracting. A color theme that doesn't distract from the main actions would improve user experience. Let's change the color of the dialog boxes to be less distracting.
New users will not know what are the features and commands of Bob. A user guide will help new users orient themselves with Bob. Let's add a new user guide that explains the features and commands of Bob.
Numbered lists do not show up as expected in github pages. Let's use code blocks in place of numbered lists instead.
Code blocks also do not show up as expected in github pages. Let's try using <br> tags instead.
While <br> tags do work in the details tag, the list is not in a monospace font. Let's try using the <pre> tag instead.
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.
No description provided.