Skip to content

[Noor Sarrah]ip - #104

Open
NoorSarrah wants to merge 29 commits into
nus-cs2113-AY2021S2:masterfrom
NoorSarrah:master
Open

[Noor Sarrah]ip#104
NoorSarrah wants to merge 29 commits into
nus-cs2113-AY2021S2:masterfrom
NoorSarrah:master

Conversation

@NoorSarrah

Copy link
Copy Markdown

No description provided.

@rafaelperes rafaelperes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good job Sarrah. Maybe you should also take a look at ways to improve your code abstraction (check the SLAP hard concept for example).

Comment thread src/main/java/Deadline.java Outdated
}
@Override
public String stringToSave() {
return "D ==> " + getStatusIcon() + " ==> " + getDescription() + " ==> " + getBy();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe you should use constants in order to reduce the need of magic strings ("==>" and etc...)

Comment thread src/main/java/Duke.java
Scanner myObj = new Scanner(System.in);
String command = myObj.nextLine();

while (!command.equals("bye")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

refer to the magic string comment

Comment thread src/main/java/Duke.java Outdated
Scanner s = new Scanner(f);
while (s.hasNext()) {
String[] task = s.nextLine().split(" ==> ");
if (task[0].equals("T")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

to improve the readability, maybe assign task[0] to some variable? e.g String command = task[0]; Same goes for task[2], tasks[3]...Quite difficulty to understand the meaning.

Comment thread src/main/java/Duke.java Outdated
}
public static void addTasks(String description) {
printDash();
if (description.contains("todo")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

refer to the magic strings comment

Comment thread src/main/java/Duke.java Outdated
printDash();
if (description.contains("todo")) {
try {
description = description.substring(5);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe there is a magic number here.

Comment thread src/main/java/List.java Outdated
@@ -0,0 +1,57 @@
/*public class List {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe you should remove all the code commented before submit your pull request.

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.

2 participants