Michael Ong iP by maikongeh · Pull Request #235 · nus-cs2103-AY2021S2/ip

maikongeh · 2021-01-28T14:12:02Z

pull request for week 3

test

Greet, Echo, Exit

add, list

Mark as done, todo, event deadline inherits tasks.

delete, error handling and running test cases

Add save function

tashawan23

Overall, I found your code quite easy to understand for the most part except for a few places where nesting was too deep. I have noted some repeated code and coding standard violations. :)

tashawan23 · 2021-02-01T13:41:53Z

src/main/java/Duke.java

+        System.out.println("____________________________________");
+
+        Scanner input = new Scanner(System.in);
+        while(input.hasNextLine()) {


Maybe it would be better if you avoid deep nesting to improve code quality

tashawan23 · 2021-02-01T13:43:44Z

src/main/java/Duke.java

+        for(String str: strings) {
+            char identifier = str.charAt(1);
+            switch(identifier) {
+                case 'D':


Maybe it would be better to leave out the indentation for case clauses

tashawan23 · 2021-02-01T13:46:30Z

src/main/java/Duke.java

+        catch (FileNotFoundException e){
+            throw new FileNotFoundException("No File Detected");
+        }
+        System.out.println("____________________________________");


Would it be better to have this line format as a constant since it is repeated several times in the class?

tashawan23 · 2021-02-01T13:50:58Z

src/main/java/Database.java

+    }
+
+    public ArrayList<String> readFile() throws FileNotFoundException {
+        try {


I like your indentations in this class!

pyuxiang

A couple of code standard comments, but otherwise adheres to standards pretty nicely!

pyuxiang · 2021-02-01T15:14:42Z

src/main/java/Database.java

+        }
+    }
+
+    public  void writeTaskToFile(List<Task> tasks){


Be careful of the spaces 😉

Suggested change

public void writeTaskToFile(List<Task> tasks){

public void writeTaskToFile(List<Task> tasks){

pyuxiang · 2021-02-01T15:19:13Z

src/main/java/Database.java

+
+    String name;
+
+


You might have inadvertently added an extra blank line here:
https://se-education.org/guides/conventions/java/intermediate.html#layout

Suggested change

pyuxiang · 2021-02-01T15:21:38Z

src/main/java/Duke.java

+        Database database = new Database( "data.txt");
+        ArrayList<String> listOfTasks;
+        ArrayList<Task> myList;
+       try {


The indentation is a little off here: should be in line with the block.

Suggested change

try {

try {

pyuxiang · 2021-02-01T15:24:13Z

src/main/java/Duke.java

+        }
+        catch (FileNotFoundException e){


The catch keyword should be in line with the closing brace.

Suggested change

}

catch (FileNotFoundException e){

} catch (FileNotFoundException e){

pyuxiang · 2021-02-01T15:25:03Z

src/main/java/Duke.java

+        System.out.println("____________________________________");
+
+        Scanner input = new Scanner(System.in);
+        while(input.hasNextLine()) {


Another space missed 😉

Suggested change

while(input.hasNextLine()) {

while (input.hasNextLine()) {

pyuxiang · 2021-02-01T15:30:44Z

src/main/java/Duke.java

+                case "todo":
+                    System.out.println("____________________________________");
+                    try {
+                        String s1 = input.nextLine();


You might have missed this: variable names could do with a more specific name, especially since the scope isn't small. Perhaps s1 can be renamed as userInputLine.

pyuxiang · 2021-02-01T15:34:07Z

src/main/java/Duke.java

+                                    database.writeTaskToFile(myList);
+
+                                }
+                            } catch(DukeException e) {


Not a coding standard, but figured I could chip in. Since the catch block is the same as that below in line 86, you can consider removing the try-except block here.

pyuxiang · 2021-02-01T15:35:11Z

src/main/java/Duke.java

+                case "list":
+                    System.out.println("____________________________________");
+                    System.out.println("Here are the tasks in your list:");
+                    for(int i=1; i< myList.size()+1; i++){


Note the spaces here as well:

Suggested change

for(int i=1; i< myList.size()+1; i++){

for (int i = 1; i < myList.size() + 1; i++) {

pyuxiang · 2021-02-01T15:40:33Z

src/main/java/Database.java

+import java.util.List;
+import java.util.Scanner;
+
+public class Database {


Consider adding a couple of class-level comments describing the class.
Might not be immediately clear otherwise what this Database class is intended for.

pyuxiang · 2021-02-01T15:43:02Z

src/main/java/Database.java

@@ -0,0 +1,47 @@
+import java.io.File;


Your import statements are very nicely formatted 🎉

Implement date function using LocalDate package

implement database - same functionality as storage, parser to handle different switch cases, Ui to handle ouput for different commands and error handling.

package files in duke

s

Introduce JavaDocs to my class and methods

Adhere to coding standards

Implemented find

Implemented gradle support

implement GUI

implement Assertions

Create User guide in ReadMe

damithc and others added 8 commits July 23, 2020 23:27

Add Gradle support

3b19ba1

build.gradle: Update version to 8.29

a75fcee

test

4498a6b

test

level1

3ba23a5

Greet, Echo, Exit

level 2

1332167

add, list

level 4

bf447d2

Mark as done, todo, event deadline inherits tasks.

level 6

dd958f6

delete, error handling and running test cases

Level 7

a4a2685

Add save function

tashawan23 reviewed Feb 1, 2021

View reviewed changes

pyuxiang reviewed Feb 1, 2021

View reviewed changes

maikongeh added 20 commits February 4, 2021 20:12

Level-8

7079a47

Implement date function using LocalDate package

A-MoreOOP

6988915

implement database - same functionality as storage, parser to handle different switch cases, Ui to handle ouput for different commands and error handling.

A-Packages

8c6a077

package files in duke

Commit before taskList

8572393

s

Implement TaskList

9dd0f7e

A-JavaDoc

b2618e0

Introduce JavaDocs to my class and methods

A-CodingStandard

2b500b2

Adhere to coding standards

Merge branch 'Branch-A-CodingStandard'

8cb2983

Level-9

ee2df78

Implemented find

Merge branch 'branch-Level-9'

fbb0793

Merge branch 'add-gradle-support'

d984a52

A-Gradle

94ecea4

Implemented gradle support

Merge branch 'branch-A-Gradle'

80cb4ab

Save Before GUI

be0283c

Level-10

57b5d4e

implement GUI

Merge branch 'branch-Level-10'

e178e21

A-Assertions

0080b9c

implement Assertions

Merge branch 'branch-Assertions'

ec0b360

A-UserGuide

b6558dc

Create User guide in ReadMe

fix bugs found

197fc2a

maikongeh and others added 14 commits February 19, 2021 19:30

update final file

80b1fea

update

7cf7620

improve codeQuality

53e6063

Merge branch 'branch-A-codeQuality'

9720822

move Ui.png to ip folder

669c7dc

Create config.yml file Set theme jekyll-theme-cayman

687d765

Merge branch 'branch-UserGuide'

27d8d64

Set theme jekyll-theme-cayman

2b332ac

Set theme jekyll-theme-cayman

37032d1

Set theme jekyll-theme-cayman

45b9fb5

Update README.md

3993d29

Merge branch 'master' of https://github.com/maikongeh/ip

78260bb

reduce long methods and deep nesting

fdd22b6

Merge branch 'master' of https://github.com/maikongeh/ip

8cbcf22

	public void writeTaskToFile(List<Task> tasks){
	public void writeTaskToFile(List<Task> tasks){

	}
	catch (FileNotFoundException e){
	} catch (FileNotFoundException e){

	for(int i=1; i< myList.size()+1; i++){
	for (int i = 1; i < myList.size() + 1; i++) {

Conversation

maikongeh commented Jan 28, 2021

Uh oh!

tashawan23 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!

pyuxiang 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!

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!

3 participants