Skip to content

[W5][M11-2]Lew Sheng Kok - #29

Open
Shengkok wants to merge 2 commits into
nusCS2113-AY1819S2:masterfrom
Shengkok:master
Open

[W5][M11-2]Lew Sheng Kok#29
Shengkok wants to merge 2 commits into
nusCS2113-AY1819S2:masterfrom
Shengkok:master

Conversation

@Shengkok

Copy link
Copy Markdown
  • Supports writing and saving notes.
  • Supports viewing of saved notes.

/** Version info of the program. */
public static final String VERSION = "AddressBook Level 2 - Version 1.0";

private static String[] NOTE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should probably have Notes as its own class instead of being in Main.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed. A proper design would require Note to be a class which associates itself to a Person.

@devamanyu devamanyu 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.

PR attempts to add the functionality of notes in AB.
Changes in the design to adhere to SE principles suggested. Kindly look at the comments and close this PR.

Also try to work on updating test cases 👍

&& this.allPersons.equals(((AddressBook) other).allPersons));
}


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probably these whitespaces are not needed.

/** Version info of the program. */
public static final String VERSION = "AddressBook Level 2 - Version 1.0";

private static String[] NOTE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed. A proper design would require Note to be a class which associates itself to a Person.

for(String str: result){
System.out.println(str);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Kindly note that this method "notes" is failing SE principles. Ways to improve would include following SLAP and SRP (printing, input, etc. all happening at the same place right now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants