Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[T4A6][T13-B1] Hong Bangwu #1653

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/seedu/addressbook/data/tag/Tagging.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package seedu.addressbook.data.tag;

import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;

public class Tagging {

Name name;
Copy link

Choose a reason for hiding this comment

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

the Tagging association should identify a person and a tag, assume the application allows different people with same name, by just using name here, you wouldn't know which person was tagged with the given tag, no? what is a better way?

Tag tag;
//isValid tells us whether the tag is still available after the change occurred.
boolean isValid;

/*
* object will be initialised whenever a tag is added or removed. the method called to do so will provide a reference to
* the tag object, the person object, as well as whether the affected tag's validity to the person.
*/
public Tagging(Tag tagAffected, Person person, boolean validity) {
name = person.getName();
isValid = validity;
tag = tagAffected;
}

public String outputTaggings() {
String output = "";
if (isValid) {
output += "+ ";
} else output += "- ";
Copy link

Choose a reason for hiding this comment

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

coding standard violation here 👎

  • even if it is a single line conditional you need to have braces
  • else should be placed between } and {


output += name.toString();
output += tag.toString();

return output;
}

}
53 changes: 53 additions & 0 deletions test/java/seedu/addressbook/parser/UtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package seedu.addressbook.parser;

import static org.junit.Assert.*;

import org.junit.Test;

import seedu.addressbook.data.person.Name;
import seedu.addressbook.common.Utils;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.*;

public class UtilsTest {

@Test
public void isAnyNullTest() {
Utils Util = new Utils();
Name name = null;

//test case 1: return true for a name that is actually a null
assertTrue(Utils.isAnyNull(name));
}

@Test
public void isAnyNullTest1() {
Copy link

Choose a reason for hiding this comment

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

test name should have 3 parts <method being tested>_<what is the test condition>_<what is the expected outcome>

try {
Utils Util = new Utils();
Name name2 = new Name("Andy Tan");
Copy link

Choose a reason for hiding this comment

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

going further you can consider moving all the steps necessary for the test to run (or preconditions) to a set up method before beginning the tests; correspondingly can put all the tear down in a method after the tests.

Name name3 = new Name("Joshua Soh");

//test case 2: return false if no nulls among objects passed in
assertFalse(Utils.isAnyNull(name2, name3));

} catch (IllegalValueException e) {
}
}

@Test
public void isAnyNullTest2() {
try {
Copy link

Choose a reason for hiding this comment

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

in general, no need to try ... catch exceptions in the test case, you can simply use throws

Utils Util = new Utils();
Name name = null;
Name name2 = new Name("Andy Tan");
Name name3 = new Name("Joshua Soh");

//test case 3: return true if one of the name object is actually null
assertTrue(Utils.isAnyNull(name, name2, name3));

} catch (IllegalValueException e) {
}
}


}