[Caesar Pang] Duke Increments by caesarpjz · Pull Request #350 · nus-cs2103-AY1920S1/duke

caesarpjz · 2019-09-02T09:59:12Z

Duke Increments

Add toolVersion block in to Gradle code sample to prevent errors.

Change file mode on `gradle` to be executable (nus-cs2103-AY1920S1#9)

…Cleaned up my code more.

# Conflicts: # src/main/java/Duke.class # src/main/java/Duke.java # src/main/java/Task.class # src/main/java/Task.java Have completed all merge conflicts. This is the final draft for lvl 7 and lvl 8.

…only halfway done.

# Conflicts: # src/main/java/Event.class # src/main/java/Event.java # src/main/java/Storage.java # src/main/java/Task.class # src/main/java/TaskList.class # src/main/java/Todo.class # src/main/java/Todo.java

# Conflicts: # src/main/java/Ui.class

LeowWB · 2019-09-04T15:33:04Z

+    /**
+     * Main method.
+     */
+    public static void main(String[] args) throws IOException{


I think this is a bad practice. You should probably catch the IOException somewhere within your code, rather than just throwing it from your main method.

LeowWB · 2019-09-04T15:34:23Z

+     */
+    @Override
+    public String toString() {
+        String date = formatDate(when);


You may want to consider storing this formatted date as a variable, so you don't have to keep converting it all the time.

LeowWB · 2019-09-04T15:35:51Z

+    protected static ArrayList<Task> taskList = new ArrayList<Task>();
+    protected static String file = "todo.txt";
+
+    public Storage(String file) {


You should consider removing this parameter, since it doesn't seem to do anything.

LeowWB · 2019-09-04T15:35:54Z

+    protected static ArrayList<Task> taskList = new ArrayList<Task>();
+    protected static String file = "todo.txt";
+
+    public Storage(String file) {


You should consider removing this parameter, since it doesn't seem to do anything.

LeowWB · 2019-09-04T15:38:22Z

+     */
+    public void addToFile(String filepath, String textToAdd) throws IOException {
+            FileWriter typer = new FileWriter(filepath, true);
+            typer.write(textToAdd + System.lineSeparator());


Have to say that I didn't know about this System.lineSeparator() function. Looks like it might be useful for a cross-platform application. Good job 👍👍

LeowWB · 2019-09-04T15:39:39Z

+     */
+    public static int countLines(String filename) throws IOException {
+        try (InputStream inputs = new BufferedInputStream(new FileInputStream(filename))) {
+            byte[] characters = new byte[1024];


Just curious, is there a reason you chose to use byte rather than char?

LeowWB · 2019-09-04T15:42:49Z

+     * does not exist but cannot be created, or cannot be opened for any other reason.
+     */
+    public void deleteCommand(String text) throws IOException {
+        int num = text.indexOf(" ");


Maybe this variable can be named better?

LeowWB · 2019-09-04T15:46:19Z

Overall, I think you did a great job with the documentation. I didn't notice any public functions missing JavaDoc comments. Good work 👍

jon-chua

Your code is very well-documented 👍
As mentioned, I think it would be a good idea to abstract out parsing-related functionality to the Parser class.
On another note, your printed messages were interesting and useful 😄
Implementing more tests would prevent regressions from being introduced in the future too!

jon-chua · 2019-09-04T16:39:26Z

+                int index = task.indexOf("[");
+                String taskType = task.substring(index + 1, index + 2);
+                int spaceIndex = task.indexOf(" ");
+                switch (taskType) {


There's a specific convention to follow for switch statements! https://nus-cs2103-ay1920s1.github.io/website/coding-standards/java/intermediate.html

jon-chua · 2019-09-04T16:40:44Z

@@ -0,0 +1,167 @@
+import java.io.IOException;
+import java.sql.SQLOutput;


This seems like an unused import.

jon-chua · 2019-09-04T16:43:16Z

+     * @throws IOException If the named file exists but is a directory rather than a regular file,
+     * does not exist but cannot be created, or cannot be opened for any other reason.
+     */
+    public void eventCommand(String text) throws DukeException, IOException {


I think the parsing of the String should be moved to the Parser class, the TaskList class should be concerned with the operations on the list itself.

jon-chua · 2019-09-04T16:43:44Z

+     * @throws IOException If the named file exists but is a directory rather than a regular file,
+     * does not exist but cannot be created, or cannot be opened for any other reason.
+     */
+    public void deadlineCommand(String text) throws DukeException, IOException {


Similar to eventCommand, the parsing can be abstracted out to the Parser class.

jon-chua · 2019-09-04T16:44:22Z

+     * command based on the input. Prints out error messages as well.
+     */
+    public void nextCommand() {
+        while (scan.hasNext()) {


I think we should try to use the Parser class you have created to parse the input first! This way we can abstract out the parsing functions and so the UI class can just be concerned with the UI tasks.

groot0x12

Hi! You might want to clean up your repository by avoiding pushing your class files or just do a repository refactor. Overall, great effort but you might want to do more javadocs and documentation because it was a bit hard to understand the intention behind your code :)

groot0x12 · 2019-09-13T01:18:53Z

@@ -0,0 +1,15 @@
+public class Command {


I feel that this is a plausible solution but you might want to consider abstract classes as a more efficient way of inheritance for all your other command sub-classes.

groot0x12 · 2019-09-13T01:20:35Z

+
+    public String nextCommand(String input) {
+        String text = input;
+        switch ()


Missing documentation and code looks incomplete. Missing a return statement as well. I'm guessing what you're trying to do here is abstract the Command class to have some form of inheritance, instead of using methods to facilitate your commands. You're on the right track, just add more to this switch statement.

groot0x12 · 2019-09-13T01:23:38Z

+    }
+
+
+    public static String dateFormatter(String text) {


This is definitely a plausible solution to obtaining substrings from the command. However, you might want to consider using the java String.split() method to make obtaining your data more efficient!

groot0x12 · 2019-09-13T01:26:11Z

+import javafx.scene.image.Image;
+import javafx.scene.image.ImageView;
+
+public class Duke extends Application {


You might want to consider using multiple classes to facilitate your JavaFX GUI (MainWindow.java / DialogBox.java etc.)
Having all the code in your Duke.java makes it very messy and unreadable.

groot0x12 · 2019-09-13T01:27:05Z

+                    ui.printIndent();
+                    switch (text) {
+                    case "todo":
+                        throw new DukeException("☹ OOPS!!! The description of a todo cannot be empty. "


Your error format can be stored in a string variable so that it's easy to reuse.

groot0x12 · 2019-09-13T01:31:18Z

+        String formatted = date;
+        if (!date.contains(")")) {
+            try {
+                Date d = new SimpleDateFormat("dd/MM/yyyy hhmm").parse(date);


Great effort in handling date and time!

Added assertions.

Better Code Quality.

j-lum and others added 27 commits August 6, 2019 15:25

Add support for Gradle workflow

65f72a8

Add sample checkstyle configuration

0112efe

Add toolVersion block in to Gradle code sample to prevent errors.

Change file mode on gradle to be executable

cfd6da7

Merge pull request nus-cs2103-AY1920S1#12 from j-lum/gradle+x

6e6ace1

Change file mode on `gradle` to be executable (nus-cs2103-AY1920S1#9)

Done Level 1. Added Greeting, Echo and Exit.

3a7a107

Done Level 2. Added List function and Task function.

68a7aea

Done lvl 3.

73a6527

Done lvl 4.

dbb4abc

Done lvl 5.

176e935

Done lvl 6.

def7477

Clean up my code a bit. Added some edge cases and fixed some issues.

c0f8da0

Added a storage class to add tasks into a file. Done a save feature. …

e89761d

…Cleaned up my code more.

Done date changes and formats.

0c6b61e

Merge branch 'branch-Level-8'

5d44998

# Conflicts: # src/main/java/Duke.class # src/main/java/Duke.java # src/main/java/Task.class # src/main/java/Task.java Have completed all merge conflicts. This is the final draft for lvl 7 and lvl 8.

Done A-MoreOOP. Added more classes.

091b79e

Cleaned up code again, Seperated them more into different class. But …

c69f71c

…only halfway done.

Done JUnit.

6ef4264

Done jar.

935d978

Done JavaDoc comments.

780cd71

did some changes.

3bad8f8

minor changes

8fd78d7

Done JavaDocs

52f5392

Done Coding Standard.

b325ed9

Done Level 9 feature (find).

49db2a4

Merge branch 'branch-A-CodingStandard'

f3c9622

# Conflicts: # src/main/java/Event.class # src/main/java/Event.java # src/main/java/Storage.java # src/main/java/Task.class # src/main/java/TaskList.class # src/main/java/Todo.class # src/main/java/Todo.java

Merge branch 'branch-Level-9'

1f1162d

# Conflicts: # src/main/java/Ui.class

Merge

7e5d619

LeowWB reviewed Sep 4, 2019

View reviewed changes

jon-chua reviewed Sep 4, 2019

View reviewed changes

caesarpjz added 3 commits September 13, 2019 00:06

Done some changes

987507f

Clean some things up

31ffff6

Added new things

723f40d

groot0x12 reviewed Sep 13, 2019

View reviewed changes

caesarpjz and others added 26 commits September 15, 2019 05:48

Did some changes. Added Command classes. Did more OOP as well

9dfc61a

Did some changes to handle input better.

20b7205

Made some changes.

72dfe57

Created and added Travis

019698e

Added D-Expenses.

d568912

Added C-Help.

6266873

Added some design and started on user guide.

1c7277a

Changes to url link

5782932

Experimenting with the width and style of logo.

5666c2b

Changes made to README.md and added images.

b02e389

Done up packaging and user guide and Ui.png.

8bed604

Set theme jekyll-theme-time-machine

1e0267f

Changes made to README.

1fe1e75

Merge branch 'master' of https://github.com/caesarpjz/duke

7d9bd97

Changes to README

0a81e39

Added some JavaDocs.

b18a292

Edited some JavaDocs.

26b0cc8

Fix test cases.

ae98f7f

fixed build.gradle

d541fae

Fixed Storage Test.

7c475c5

Minor mistake in storage file

b5a4861

Did some edits to allow shadowJar to work.

9ace65c

Added assertions.

ca838a8

Better Code Quality.

6222066

Merge pull request #5 from caesarpjz/branch-A-Assertions

c3a2655

Added assertions.

Merge pull request #6 from caesarpjz/branch-A-CodeQuality

28874c8

Better Code Quality.

		@@ -0,0 +1,167 @@
		import java.io.IOException;
		import java.sql.SQLOutput;

Conversation

caesarpjz commented Sep 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeowWB commented Sep 4, 2019

Uh oh!

jon-chua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

groot0x12 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants