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

Commit

Permalink
Refactor asserts used for parameter validation
Browse files Browse the repository at this point in the history
Using asserts to validate the state of parameters will result in a
generic AssertionError. This is not desirable as assertions should
not be used for parameter validation. It should be enforced by
explicit checks that throw specific exceptions.

Let's refactor the methods to better comply with these new guidelines.
  • Loading branch information
Zhiyuan-Amos committed May 15, 2017
1 parent 684310f commit 1087d98
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.commons.core;

import static com.google.common.base.Preconditions.checkNotNull;

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

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

Expand Down
4 changes: 3 additions & 1 deletion 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 com.google.common.base.Preconditions.checkArgument;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -80,7 +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.contains("/");
checkArgument(pathWithForwardSlash.contains("/"));
return pathWithForwardSlash.replace("/", File.separator);
}

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

import static com.google.common.base.Preconditions.checkArgument;

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

Expand All @@ -21,8 +23,8 @@ public class StringUtil {
*/
public static boolean containsWordIgnoreCase(String sentence, String 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 Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package seedu.address.logic.commands;

import static com.google.common.base.Preconditions.checkNotNull;

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

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

Expand Down
14 changes: 8 additions & 6 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 com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
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,7 +55,7 @@ public class EditCommand extends Command {
* @param editPersonDescriptor details to edit the person with
*/
public EditCommand(int filteredPersonListIndex, EditPersonDescriptor editPersonDescriptor) {
assert filteredPersonListIndex > 0;
checkArgument(filteredPersonListIndex > 0);

this.filteredPersonListIndex = IndexUtil.oneToZeroIndex(filteredPersonListIndex);
this.editPersonDescriptor = new EditPersonDescriptor(editPersonDescriptor);
Expand Down Expand Up @@ -125,7 +127,7 @@ public boolean isAnyFieldEdited() {
}

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

Expand All @@ -134,7 +136,7 @@ public Optional<Name> getName() {
}

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

Expand All @@ -143,7 +145,7 @@ public Optional<Phone> getPhone() {
}

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

Expand All @@ -152,7 +154,7 @@ public Optional<Email> getEmail() {
}

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

Expand All @@ -161,7 +163,7 @@ public Optional<Address> getAddress() {
}

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

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

import static com.google.common.base.Preconditions.checkArgument;

import java.util.Set;
import java.util.logging.Logger;

Expand Down Expand Up @@ -30,7 +32,7 @@ public class ModelManager extends ComponentManager implements Model {
*/
public ModelManager(ReadOnlyAddressBook addressBook, UserPrefs userPrefs) {
super();
assert !CollectionUtil.isAnyNull(addressBook, userPrefs);
checkArgument(!CollectionUtil.isAnyNull(addressBook, userPrefs));

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

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

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import java.util.Collections;
import java.util.Objects;
import java.util.Set;
Expand All @@ -25,7 +28,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) {
assert !CollectionUtil.isAnyNull(name, phone, email, address, tags);
checkArgument(!CollectionUtil.isAnyNull(name, phone, email, address, tags));
this.name = name;
this.phone = phone;
this.email = email;
Expand All @@ -42,7 +45,7 @@ public Person(ReadOnlyPerson source) {
}

public void setName(Name name) {
assert name != null;
checkNotNull(name);
this.name = name;
}

Expand All @@ -52,7 +55,7 @@ public Name getName() {
}

public void setPhone(Phone phone) {
assert phone != null;
checkNotNull(phone);
this.phone = phone;
}

Expand All @@ -62,7 +65,7 @@ public Phone getPhone() {
}

public void setEmail(Email email) {
assert email != null;
checkNotNull(email);
this.email = email;
}

Expand All @@ -72,7 +75,7 @@ public Email getEmail() {
}

public void setAddress(Address address) {
assert address != null;
checkNotNull(address);
this.address = address;
}

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

import static com.google.common.base.Preconditions.checkArgument;

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

Expand Down Expand Up @@ -99,7 +101,7 @@ public void setTags(UniqueTagList replacement) {
}

public void setTags(Collection<Tag> tags) throws DuplicateTagException {
assert !CollectionUtil.isAnyNull(tags);
checkArgument(!CollectionUtil.isAnyNull(tags));
if (!CollectionUtil.elementsAreUnique(tags)) {
throw new DuplicateTagException();
}
Expand Down

0 comments on commit 1087d98

Please sign in to comment.