Стиль конструктора Java: проверьте, что параметры не являются пустыми
Что является лучшими практиками, если у Вас есть класс, который принимает некоторые параметры, но ни одному из них не позволяют быть null?
Следующее очевидно, но исключение немного неконкретно:
public class SomeClass
{
public SomeClass(Object one, Object two)
{
if (one == null || two == null)
{
throw new IllegalArgumentException("Parameters can't be null");
}
//...
}
}
Здесь исключения сообщают, какой параметр является пустым, но конструктор теперь довольно уродлив:
public class SomeClass
{
public SomeClass(Object one, Object two)
{
if (one == null)
{
throw new IllegalArgumentException("one can't be null");
}
if (two == null)
{
throw new IllegalArgumentException("two can't be null");
}
//...
}
Здесь конструктор более опрятен, но теперь код конструктора не находится действительно в конструкторе:
public class SomeClass
{
public SomeClass(Object one, Object two)
{
setOne(one);
setTwo(two);
}
public void setOne(Object one)
{
if (one == null)
{
throw new IllegalArgumentException("one can't be null");
}
//...
}
public void setTwo(Object two)
{
if (two == null)
{
throw new IllegalArgumentException("two can't be null");
}
//...
}
}
Какой из этих стилей является лучшим?
Или есть ли альтернатива, которая более широко принята?
7 ответов
Второй или третий.
Потому что это говорит пользователю вашего API, что именно пошло не так.
Для меньшей многословности используйте Validate.notNull(obj, message) из commons-lang. Таким образом, ваш конструктор будет выглядеть так:
public SomeClass(Object one, Object two) {
Validate.notNull(one, "one can't be null");
Validate.notNull(two, "two can't be null");
...
}
Размещение проверки в сеттере также допустимо, с таким же многословным комментарием. Если ваши сеттеры также должны сохранять согласованность объектов, вы можете выбрать и третий вариант.
Аннотации для статического анализа также полезны как в дополнение к проверкам во время выполнения, так и вместо них.
FindBugs, например, предоставляет аннотацию @NonNull.
public SomeClass (@NonNull Object one, @NonNull Object two) {
Я бы сделал полезный метод:
public static <T> T checkNull(String message, T object) {
if(object == null) {
throw new NullPointerException(message);
}
return object;
}
Я бы сделал так, чтобы он возвращал объект, чтобы вы могли использовать его в заданиях вроде этого:
public Constructor(Object param) {
this.param = checkNull("Param not allowed to be null", param);
}
EDIT: Что касается предложений использовать сторонние библиотеки, то Google Preconditions, в частности, делает все вышеперечисленное даже лучше, чем мой код. Однако, если это единственная причина для включения библиотеки в ваш проект, я бы засомневался. Метод слишком прост.
Вы можете использовать одну из многих библиотек, разработанных для облегчения проверки предусловий. Многие коды в Google Guava используют com.google.common.base.Preconditions
Простые статические методы, вызываемые в начале ваших собственных методов для проверки правильности аргументов и состояния. Это позволяет использовать такие конструкции, как
if (count <= 0) { throw new IllegalArgumentException("должно быть положительным: " + count); }заменить на более компактную
checkArgument(count > 0, "must be positive: %s", count);
В нем есть checkNotNull который широко используется в Guava. Затем вы можете написать:
import static com.google.common.base.Preconditions.checkNotNull;
//...
public SomeClass(Object one, Object two) {
this.one = checkNotNull(one);
this.two = checkNotNull(two, "two can't be null!");
//...
}
Большинство методов перегружены, чтобы либо не принимать сообщение об ошибке, либо принимать фиксированное сообщение об ошибке, либо шаблонизированное сообщение об ошибке с varargs.
Об IllegalArgumentException против NullPointerException
В то время как ваш исходный код выбрасывает IllegalArgumentException на null аргументы, Preconditions. checkNotNull выбрасывает NullPointerException вместо этого.
Вот цитата из Effective Java 2nd Edition: Item 60: Favor the use of standard exceptions:
Возможно, все ошибочные вызовы методов сводятся к недопустимому аргументу или недопустимому состоянию, но другие исключения стандартно используются для определенных видов недопустимых аргументов и состояний. Если вызывающая сторона передает
nullв каком-то параметре, для которого запрещены нулевые значения, то по правилам должен быть выброшенNullPointerException, а неIllegalArgumentException.
Исключение NullPointerException не предназначено только для случаев, когда вы обращаетесь к членам ссылки null; это довольно стандартный случай, когда аргумент имеет значение null, когда это недопустимое значение.
System.out.println("some string".split(null));
// throws NullPointerException
Вы можете просто иметь метод, который принимает все аргументы конструктора, которые вам нужно проверить. Этот метод генерирует исключение с определенным сообщением в зависимости от того, какой аргумент недействителен. Ваш конструктор вызывает этот метод и, если он проходит, инициализирует значения.
Помимо приведенных выше ответов, которые являются действительными и разумными, я думаю, хорошо отметить, что, возможно, проверка на null не является необходимой «хорошей практикой». (Предполагая, что читатели, не являющиеся OP, могут воспринять этот вопрос как догматический)
Из блога Misko Hevery о проверяемости: Утверждать или не утверждать
Альтернативой генерации непроверяемого исключения может быть использование assert . В противном случае я бы выбрасывал проверенные исключения, чтобы вызывающая сторона знала о том, что конструктор не будет работать с недопустимыми значениями.
Разница между вашими первыми двумя решениями - нужно ли вам подробное сообщение об ошибке, нужно ли вам знать, какой параметр дал сбой, или достаточно знать, что экземпляр не мог быть создан из-за недопустимых аргументов?
Обратите внимание, что второй и третий примеры не могут правильно сообщить, что оба параметра были нулевыми.
Кстати - я голосую за вариант (1):
if (one == null || two == null) {
throw new IllegalArgumentException(
String.format("Parameters can't be null: one=%s, two=%s", one, two));
}