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

Commit

Permalink
[#239] Reduce unnecessary asserts (#414)
Browse files Browse the repository at this point in the history
Parameter validation is done by using asserts, and throws AssertionError
when validation fails.

Asserts may not always be enabled. Furthermore, specific exceptions
such as NullPointerException should be thrown to make the error more
explicit.

Let's
    - Replace the asserts for public APIs with checkNotNull() and
      checkArgument(), which throws an appropriate exception when 
      validation fails.
    - Enhance CollectionUtil#isAnyNull(…) to do the same operation as
      Objects#requireNonNull(T), and also able to take in multiple 
      parameters.
  • Loading branch information
yamgent authored Jun 1, 2017
2 parents 3e929d6 + 288a958 commit 2f5b10f
Show file tree
Hide file tree
Showing 37 changed files with 280 additions and 185 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.commons.core;

import static java.util.Objects.requireNonNull;

import java.text.Collator;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -28,8 +30,7 @@ public class UnmodifiableObservableList<E> implements ObservableList<E> {
private final ObservableList<? extends E> backingList;

public UnmodifiableObservableList(ObservableList<? extends E> backingList) {
assert backingList != null;
this.backingList = backingList;
this.backingList = requireNonNull(backingList);
}

@Override
Expand Down
25 changes: 24 additions & 1 deletion src/main/java/seedu/address/commons/util/AppUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.commons.util;

import static java.util.Objects.requireNonNull;

import javafx.scene.image.Image;
import seedu.address.MainApp;

Expand All @@ -9,8 +11,29 @@
public class AppUtil {

public static Image getImage(String imagePath) {
assert imagePath != null;
requireNonNull(imagePath);
return new Image(MainApp.class.getResourceAsStream(imagePath));
}

/**
* Checks that {@code condition} is true. Used for validating arguments to methods.
*
* @throws IllegalArgumentException if {@code condition} is false.
*/
public static void checkArgument(Boolean condition) {
if (!condition) {
throw new IllegalArgumentException();
}
}

/**
* Checks that {@code condition} is true. Used for validating arguments to methods.
*
* @throws IllegalArgumentException with {@code errorMessage} if {@code condition} is false.
*/
public static void checkArgument(Boolean condition, String errorMessage) {
if (!condition) {
throw new IllegalArgumentException(errorMessage);
}
}
}
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 #requireAllNonNull(Collection) */
public static void requireAllNonNull(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 requireAllNonNull(Collection<?> items) {
Objects.requireNonNull(items);
items.forEach(Objects::requireNonNull);
}

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

import static seedu.address.commons.util.AppUtil.checkArgument;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -80,8 +82,7 @@ public static void writeToFile(File file, String content) throws IOException {
* @return {@code pathWithForwardSlash} but '/' replaced with {@code File.separator}
*/
public static String getPath(String pathWithForwardSlash) {
assert pathWithForwardSlash != null;
assert pathWithForwardSlash.contains("/");
checkArgument(pathWithForwardSlash.contains("/"));
return pathWithForwardSlash.replace("/", File.separator);
}

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/seedu/address/commons/util/JsonUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.commons.util;

import static java.util.Objects.requireNonNull;

import java.io.File;
import java.io.IOException;
import java.util.Optional;
Expand Down Expand Up @@ -54,9 +56,7 @@ static <T> T deserializeObjectFromJsonFile(File jsonFile, Class<T> classOfObject
*/
public static <T> Optional<T> readJsonFile(
String filePath, Class<T> classOfObjectToDeserialize) throws DataConversionException {

assert filePath != null;

requireNonNull(filePath);
File file = new File(filePath);

if (!file.exists()) {
Expand Down Expand Up @@ -84,8 +84,8 @@ public static <T> Optional<T> readJsonFile(
* @throws IOException if there was an error during writing to the file
*/
public static <T> void saveJsonFile(T jsonFile, String filePath) throws IOException {
assert jsonFile != null;
assert filePath != null;
requireNonNull(filePath);
requireNonNull(jsonFile);

serializeObjectToJsonFile(new File(filePath), jsonFile);
}
Expand Down
13 changes: 8 additions & 5 deletions src/main/java/seedu/address/commons/util/StringUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package seedu.address.commons.util;

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

import java.io.PrintWriter;
import java.io.StringWriter;

Expand All @@ -20,12 +23,12 @@ public class StringUtil {
* @param word cannot be null, cannot be empty, must be a single word
*/
public static boolean containsWordIgnoreCase(String sentence, String word) {
assert word != null : "Word parameter cannot be null";
assert sentence != null : "Sentence parameter cannot be null";
requireNonNull(sentence);
requireNonNull(word);

String preppedWord = word.trim();
assert !preppedWord.isEmpty() : "Word parameter cannot be empty";
assert preppedWord.split("\\s+").length == 1 : "Word parameter should be a single word";
checkArgument(!preppedWord.isEmpty(), "Word parameter cannot be empty");
checkArgument(preppedWord.split("\\s+").length == 1, "Word parameter should be a single word");

String preppedSentence = sentence;
String[] wordsInPreppedSentence = preppedSentence.split("\\s+");
Expand All @@ -42,7 +45,7 @@ public static boolean containsWordIgnoreCase(String sentence, String word) {
* Returns a detailed message of the t, including the stack trace.
*/
public static String getDetails(Throwable t) {
assert t != null;
requireNonNull(t);
StringWriter sw = new StringWriter();
t.printStackTrace(new PrintWriter(sw));
return t.getMessage() + "\n" + sw.toString();
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/seedu/address/commons/util/XmlUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.commons.util;

import static java.util.Objects.requireNonNull;

import java.io.File;
import java.io.FileNotFoundException;

Expand Down Expand Up @@ -27,8 +29,8 @@ public class XmlUtil {
public static <T> T getDataFromFile(File file, Class<T> classToConvert)
throws FileNotFoundException, JAXBException {

assert file != null;
assert classToConvert != null;
requireNonNull(file);
requireNonNull(classToConvert);

if (!FileUtil.isFileExists(file)) {
throw new FileNotFoundException("File not found : " + file.getAbsolutePath());
Expand All @@ -51,8 +53,8 @@ public static <T> T getDataFromFile(File file, Class<T> classToConvert)
*/
public static <T> void saveDataToFile(File file, T data) throws FileNotFoundException, JAXBException {

assert file != null;
assert data != null;
requireNonNull(file);
requireNonNull(data);

if (!file.exists()) {
throw new FileNotFoundException("File not found : " + file.getAbsolutePath());
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
Expand Down Expand Up @@ -44,7 +45,7 @@ public AddCommand(ReadOnlyPerson person) {

@Override
public CommandResult execute() throws CommandException {
assert model != null;
requireNonNull(model);
try {
model.addPerson(toAdd);
return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/seedu/address/logic/commands/ClearCommand.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;

import seedu.address.model.AddressBook;

/**
Expand All @@ -13,7 +15,7 @@ public class ClearCommand extends Command {

@Override
public CommandResult execute() {
assert model != null;
requireNonNull(model);
model.resetData(new AddressBook());
return new CommandResult(MESSAGE_SUCCESS);
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/seedu/address/logic/commands/CommandResult.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;

/**
* Represents the result of a command execution.
*/
Expand All @@ -8,8 +10,7 @@ public class CommandResult {
public final String feedbackToUser;

public CommandResult(String feedbackToUser) {
assert feedbackToUser != null;
this.feedbackToUser = feedbackToUser;
this.feedbackToUser = requireNonNull(feedbackToUser);
}

}
21 changes: 9 additions & 12 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;
import static seedu.address.commons.util.AppUtil.checkArgument;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
Expand Down Expand Up @@ -53,8 +55,8 @@ public class EditCommand extends Command {
* @param editPersonDescriptor details to edit the person with
*/
public EditCommand(int filteredPersonListIndex, EditPersonDescriptor editPersonDescriptor) {
assert filteredPersonListIndex > 0;
assert editPersonDescriptor != null;
checkArgument(filteredPersonListIndex > 0);
requireNonNull(editPersonDescriptor);

this.filteredPersonListIndex = IndexUtil.oneToZeroIndex(filteredPersonListIndex);
this.editPersonDescriptor = new EditPersonDescriptor(editPersonDescriptor);
Expand Down Expand Up @@ -144,44 +146,39 @@ public boolean isAnyFieldEdited() {
}

public void setName(Optional<Name> name) {
assert name != null;
this.name = name;
this.name = requireNonNull(name);
}

public Optional<Name> getName() {
return name;
}

public void setPhone(Optional<Phone> phone) {
assert phone != null;
this.phone = phone;
this.phone = requireNonNull(phone);
}

public Optional<Phone> getPhone() {
return phone;
}

public void setEmail(Optional<Email> email) {
assert email != null;
this.email = email;
this.email = requireNonNull(email);
}

public Optional<Email> getEmail() {
return email;
}

public void setAddress(Optional<Address> address) {
assert address != null;
this.address = address;
this.address = requireNonNull(address);
}

public Optional<Address> getAddress() {
return address;
}

public void setTags(Optional<Set<Tag>> tags) {
assert tags != null;
this.tags = tags;
this.tags = requireNonNull(tags);
}

public Optional<Set<Tag>> getTags() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package seedu.address.logic.parser;

import static java.util.Objects.requireNonNull;
import static seedu.address.commons.core.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
Expand Down Expand Up @@ -29,7 +30,7 @@ public class EditCommandParser {
* @throws ParseException if the user input does not conform the expected format
*/
public EditCommand parse(String args) throws ParseException {
assert args != null;
requireNonNull(args);
ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_TAG);
List<Optional<String>> preambleFields = ParserUtil.splitPreamble(argMultimap.getPreamble(), 2);
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.logic.parser;

import static java.util.Objects.requireNonNull;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
Expand Down Expand Up @@ -49,39 +51,39 @@ public static List<Optional<String>> splitPreamble(String preamble, int numField
* Parses a {@code Optional<String> name} into an {@code Optional<Name>} if {@code name} is present.
*/
public static Optional<Name> parseName(Optional<String> name) throws IllegalValueException {
assert name != null;
requireNonNull(name);
return name.isPresent() ? Optional.of(new Name(name.get())) : Optional.empty();
}

/**
* Parses a {@code Optional<String> phone} into an {@code Optional<Phone>} if {@code phone} is present.
*/
public static Optional<Phone> parsePhone(Optional<String> phone) throws IllegalValueException {
assert phone != null;
requireNonNull(phone);
return phone.isPresent() ? Optional.of(new Phone(phone.get())) : Optional.empty();
}

/**
* Parses a {@code Optional<String> address} into an {@code Optional<Address>} if {@code address} is present.
*/
public static Optional<Address> parseAddress(Optional<String> address) throws IllegalValueException {
assert address != null;
requireNonNull(address);
return address.isPresent() ? Optional.of(new Address(address.get())) : Optional.empty();
}

/**
* Parses a {@code Optional<String> email} into an {@code Optional<Email>} if {@code email} is present.
*/
public static Optional<Email> parseEmail(Optional<String> email) throws IllegalValueException {
assert email != null;
requireNonNull(email);
return email.isPresent() ? Optional.of(new Email(email.get())) : Optional.empty();
}

/**
* Parses {@code Collection<String> tags} into a {@code Set<Tag>}.
*/
public static Set<Tag> parseTags(Collection<String> tags) throws IllegalValueException {
assert tags != null;
requireNonNull(tags);
final Set<Tag> tagSet = new HashSet<>();
for (String tagName : tags) {
tagSet.add(new Tag(tagName));
Expand Down
Loading

0 comments on commit 2f5b10f

Please sign in to comment.