Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Commit

Permalink
Update CollectionUtil#isAnyNull(Object...) to throw NPE
Browse files Browse the repository at this point in the history
Java native Objects#requireNonNull(T) can only take in an Object. Calling
the native method requires multiple calls in cases where certain methods
require multiple parameters to be non-null.

Let’s update CollectionUtil#isAnyNull(…) to do the same operation as
Objects#requireNonNull(T), and also able to take in multiple parameters.
  • Loading branch information
Zhiyuan-Amos committed May 29, 2017
1 parent 28c175b commit a4fa2db
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 58 deletions.
15 changes: 8 additions & 7 deletions src/main/java/seedu/address/commons/util/CollectionUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@
*/
public class CollectionUtil {

/** @see #isAnyNull(Collection) */
public static boolean isAnyNull(Object... items) {
return Stream.of(items).anyMatch(Objects::isNull);
/** @see #requireNonNull(Collection) */
public static void requireNonNull(Object... items) {
Objects.requireNonNull(items);
Stream.of(items).forEach(Objects::requireNonNull);
}

/**
* Returns true if any element of {@code items} is null.
* @throws NullPointerException if {@code items} itself is null.
* Throws NullPointerException if {@code items} or any element of {@code items} is null.
*/
public static boolean isAnyNull(Collection<?> items) {
return items.stream().anyMatch(Objects::isNull);
public static void requireNonNull(Collection<?> items) {
Objects.requireNonNull(items);
items.forEach(Objects::requireNonNull);
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/seedu/address/model/ModelManager.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package seedu.address.model;

import static java.util.Objects.requireNonNull;
import static seedu.address.commons.util.AppUtil.checkArgument;
import static seedu.address.commons.util.CollectionUtil.requireNonNull;

import java.util.Set;
import java.util.logging.Logger;
Expand All @@ -11,7 +11,6 @@
import seedu.address.commons.core.LogsCenter;
import seedu.address.commons.core.UnmodifiableObservableList;
import seedu.address.commons.events.model.AddressBookChangedEvent;
import seedu.address.commons.util.CollectionUtil;
import seedu.address.commons.util.StringUtil;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.UniquePersonList;
Expand All @@ -32,7 +31,7 @@ public class ModelManager extends ComponentManager implements Model {
*/
public ModelManager(ReadOnlyAddressBook addressBook, UserPrefs userPrefs) {
super();
checkArgument(!CollectionUtil.isAnyNull(addressBook, userPrefs));
requireNonNull(addressBook, userPrefs);

logger.fine("Initializing with address book: " + addressBook + " and user prefs " + userPrefs);

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/seedu/address/model/person/Person.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package seedu.address.model.person;

import static java.util.Objects.requireNonNull;
import static seedu.address.commons.util.AppUtil.checkArgument;
import static seedu.address.commons.util.CollectionUtil.requireNonNull;

import java.util.Collections;
import java.util.Objects;
import java.util.Set;

import seedu.address.commons.util.CollectionUtil;
import seedu.address.model.tag.Tag;
import seedu.address.model.tag.UniqueTagList;

Expand All @@ -28,7 +27,7 @@ public class Person implements ReadOnlyPerson {
* Every field must be present and not null.
*/
public Person(Name name, Phone phone, Email email, Address address, Set<Tag> tags) {
checkArgument(!CollectionUtil.isAnyNull(name, phone, email, address, tags));
requireNonNull(name, phone, email, address, tags);
this.name = name;
this.phone = phone;
this.email = email;
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/model/tag/UniqueTagList.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package seedu.address.model.tag;

import static java.util.Objects.requireNonNull;
import static seedu.address.commons.util.AppUtil.checkArgument;
import static seedu.address.commons.util.CollectionUtil.requireNonNull;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -54,7 +54,7 @@ public UniqueTagList(String... tags) throws DuplicateTagException, IllegalValueE
* Enforces no nulls or duplicates.
*/
public UniqueTagList(Tag... tags) throws DuplicateTagException {
checkArgument(!CollectionUtil.isAnyNull((Object[]) tags));
requireNonNull((Object[]) tags);
final List<Tag> initialTags = Arrays.asList(tags);
if (!CollectionUtil.elementsAreUnique(initialTags)) {
throw new DuplicateTagException();
Expand All @@ -80,7 +80,7 @@ public UniqueTagList(Collection<Tag> tags) throws DuplicateTagException {
* Enforces no nulls.
*/
public UniqueTagList(Set<Tag> tags) {
checkArgument(!CollectionUtil.isAnyNull(tags));
requireNonNull(tags);
internalList.addAll(tags);

assert CollectionUtil.elementsAreUnique(internalList);
Expand Down Expand Up @@ -114,7 +114,7 @@ public void setTags(UniqueTagList replacement) {
}

public void setTags(Collection<Tag> tags) throws DuplicateTagException {
checkArgument(!CollectionUtil.isAnyNull(tags));
requireNonNull(tags);
if (!CollectionUtil.elementsAreUnique(tags)) {
throw new DuplicateTagException();
}
Expand Down
99 changes: 58 additions & 41 deletions src/test/java/seedu/address/commons/util/CollectionUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,84 +2,75 @@

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static seedu.address.commons.util.CollectionUtil.requireNonNull;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class CollectionUtilTest {
@Rule
public ExpectedException thrown = ExpectedException.none();

@Test
public void isAnyNullVarargs() {
public void requireNonNullVarargs() {
// no arguments
assertFalse(CollectionUtil.isAnyNull());
assertNullPointerExceptionNotThrown();

// any non-empty argument list
assertFalse(CollectionUtil.isAnyNull(new Object(), new Object()));
assertFalse(CollectionUtil.isAnyNull("test"));
assertFalse(CollectionUtil.isAnyNull(""));
assertNullPointerExceptionNotThrown(new Object(), new Object());
assertNullPointerExceptionNotThrown("test");
assertNullPointerExceptionNotThrown("");

// argument lists with just one null at the beginning
assertTrue(CollectionUtil.isAnyNull((Object) null));
assertTrue(CollectionUtil.isAnyNull(null, "", new Object()));
assertTrue(CollectionUtil.isAnyNull(null, new Object(), new Object()));
assertNullPointerExceptionThrown((Object) null);
assertNullPointerExceptionThrown(null, "", new Object());
assertNullPointerExceptionThrown(null, new Object(), new Object());

// argument lists with nulls in the middle
assertTrue(CollectionUtil.isAnyNull(new Object(), null, null, "test"));
assertTrue(CollectionUtil.isAnyNull("", null, new Object()));
assertNullPointerExceptionThrown(new Object(), null, null, "test");
assertNullPointerExceptionThrown("", null, new Object());

// argument lists with one null as the last argument
assertTrue(CollectionUtil.isAnyNull("", new Object(), null));
assertTrue(CollectionUtil.isAnyNull(new Object(), new Object(), null));
assertNullPointerExceptionThrown("", new Object(), null);
assertNullPointerExceptionThrown(new Object(), new Object(), null);

// null reference
assertNullPointerExceptionThrown((Object[]) null);

// confirms nulls inside lists in the argument list are not considered
List<Object> containingNull = Arrays.asList((Object) null);
assertFalse(CollectionUtil.isAnyNull(containingNull, new Object()));
}

@Test
public void isAnyNullVarargs_nullReference_throwsNullPointerException() {
thrown.expect(NullPointerException.class);
CollectionUtil.isAnyNull((Object[]) null);
assertNullPointerExceptionNotThrown(containingNull, new Object());
}

@Test
public void isAnyNullCollection() {
public void requireNonNullCollection() {
// lists containing nulls in the front
assertTrue(CollectionUtil.isAnyNull(Arrays.asList((Object) null)));
assertTrue(CollectionUtil.isAnyNull(Arrays.asList(null, new Object(), "")));
assertNullPointerExceptionThrown(Arrays.asList((Object) null));
assertNullPointerExceptionThrown(Arrays.asList(null, new Object(), ""));

// lists containing nulls in the middle
assertTrue(CollectionUtil.isAnyNull(Arrays.asList("spam", null, new Object())));
assertTrue(CollectionUtil.isAnyNull(Arrays.asList("spam", null, "eggs", null, new Object())));
assertNullPointerExceptionThrown(Arrays.asList("spam", null, new Object()));
assertNullPointerExceptionThrown(Arrays.asList("spam", null, "eggs", null, new Object()));

// lists containing nulls at the end
assertTrue(CollectionUtil.isAnyNull(Arrays.asList("spam", new Object(), null)));
assertTrue(CollectionUtil.isAnyNull(Arrays.asList(new Object(), null)));
assertNullPointerExceptionThrown(Arrays.asList("spam", new Object(), null));
assertNullPointerExceptionThrown(Arrays.asList(new Object(), null));

// null reference
assertNullPointerExceptionThrown((Collection<Object>) null);

// empty list
assertFalse(CollectionUtil.isAnyNull(Collections.emptyList()));
assertNullPointerExceptionNotThrown(Collections.emptyList());

// list with all non-null elements
assertFalse(CollectionUtil.isAnyNull(Arrays.asList(new Object(), "ham", new Integer(1))));
assertFalse(CollectionUtil.isAnyNull(Arrays.asList(new Object())));
assertNullPointerExceptionNotThrown(Arrays.asList(new Object(), "ham", new Integer(1)));
assertNullPointerExceptionNotThrown(Arrays.asList(new Object()));

// confirms nulls inside nested lists are not considered
List<Object> containingNull = Arrays.asList((Object) null);
assertFalse(CollectionUtil.isAnyNull(Arrays.asList(containingNull, new Object())));
}

@Test
public void isAnyNullCollection_nullReference_throwsNullPointerException() {
thrown.expect(NullPointerException.class);
CollectionUtil.isAnyNull((Collection<Object>) null);
assertNullPointerExceptionNotThrown(Arrays.asList(containingNull, new Object()));
}

@Test
Expand Down Expand Up @@ -107,6 +98,32 @@ public void elementsAreUnique() throws Exception {
assertNotUnique(null, "a", "b", null);
}

private void assertNullPointerExceptionThrown(Object... objects) {
try {
requireNonNull(objects);
fail("Expected NullPointerException was not thrown");
} catch (NullPointerException npe) {
// expected behavior
}
}

private void assertNullPointerExceptionThrown(Collection<?> objects) {
try {
requireNonNull(objects);
fail("Expected NullPointerException was not thrown");
} catch (NullPointerException npe) {
// expected behavior
}
}

private void assertNullPointerExceptionNotThrown(Object... objects) {
requireNonNull(objects);
}

private void assertNullPointerExceptionNotThrown(Collection<?> objects) {
requireNonNull(objects);
}

private void assertAreUnique(Object... objects) {
assertTrue(CollectionUtil.elementsAreUnique(Arrays.asList(objects)));
}
Expand Down

0 comments on commit a4fa2db

Please sign in to comment.