Проверьте два аргумента в Java, либо оба не равны NULL, либо оба NULL элегантно

161

Я использовал Spring Boot для разработки проекта оболочки, используемого для отправки электронной почты, например

sendmail -from foo@bar.com -password  foobar -subject "hello world"  -to aaa@bbb.com

Если fromи passwordаргументы отсутствуют, я использую отправитель по умолчанию и пароль, например , noreply@bar.comи 123456.

Таким образом, если пользователь передает fromаргумент, он также должен передать passwordаргумент и наоборот. То есть, либо оба ненулевые, либо оба нулевые.

Как я могу проверить это элегантно?

Теперь мой путь

if ((from != null && password == null) || (from == null && password != null)) {
    throw new RuntimeException("from and password either both exist or both not exist");
}
zhuguowei
источник
14
Кроме того, обратите внимание, как осторожно использование пробелов делает код намного проще для чтения - простое добавление пробелов между операторами в текущем коде значительно повысит читабельность IMO.
Джон Скит
8
Пожалуйста, определите «элегантность».
Рено
Вам нужен отдельный набор аргументов для учетных данных аутентификации SMTP и для адреса электронной почты отправителя конверта. FromЭлектронная почта не всегда имя аутентификации SMTP.
Каз
3
Это действительно не может быть намного более оптимизировано, это одна строка читаемого кода, и ничего нельзя получить путем чрезмерной оптимизации.
Дийневала
3
Примечание: если это сценарий оболочки, не сохранятся ли пароли в истории оболочки?
дотронься до моего тела

Ответы:

331

Есть способ с помощью оператора ^( XOR ):

if (from == null ^ password == null) {
    // Use RuntimeException if you need to
    throw new IllegalArgumentException("message");
}

ifУсловие будет верным , если только одна переменная равна нулю.

Но я думаю, что обычно лучше использовать два ifусловия с разными сообщениями об исключениях. Вы не можете определить, что пошло не так, используя одно условие.

if ((from == null) && (password != null)) {
    throw new IllegalArgumentException("If from is null, password must be null");
}
if ((from != null) && (password == null)) {
    throw new IllegalArgumentException("If from is not null, password must not be null");
}

Он более читабелен и намного проще для понимания, и для этого требуется лишь немного больше печатать.

клевый парень
источник
152
Есть ли причина, по которой xor на двух bools предпочтительнее, чем !=на двух bools?
Эрик Липперт
1
Вау, я не понял, что XOR для логического значения такой же, как !=. Mindblown. И это очень большое количество голосов в комментарии. И, чтобы повысить качество этого комментария, да, я также считаю, что лучше давать разные сообщения об ошибках для разных случаев ошибки, так как пользователь будет лучше знать, как исправить ошибку.
justhalf
1
Для 2 элементов это хорошо. Как это сделать с> 2 элементами?
Анушри Ахарджи
Я хочу показать сообщение об ошибке, если любой из элементов> 0. По умолчанию все установлены на 0. Если все> 0, это допустимый сценарий. Как это сделать?
Анушри Ахарджи
Это сделало бы отличный вопрос для интервью. Интересно, какой процент программистов это знает. Но я не согласен с тем, что это недостаточно ясно и требует разделения на 2 ifпункта - сообщение в исключении хорошо
Адам
286

Ну, звучит так, будто вы пытаетесь проверить, является ли условие "недействительности" двух одинаковым или нет. Вы можете использовать:

if ((from == null) != (password == null))
{
    ...
}

Или сделайте это более явным с помощью вспомогательных переменных:

boolean gotFrom = from != null;
boolean gotPassword = password != null;
if (gotFrom != gotPassword)
{
    ...
}
Джон Скит
источник
18
Ты один из моих ТАКИХ героев @Kaz, но ничто не говорит «или то, или это, но не то же самое», как это ^делает. :-)
Дэвид Баллок
6
@DavidBullock: когда речь идет о логических значениях, ничто не говорит «или то или это, но не оба одинаковые» как ... «не оба одинаковые»; XOR - это просто другое имя функции «не равно» над логическими значениями.
Каз
5
@Kaz Это просто ^говорит: «Эй, мои операнды - логические выражения», но !=это не так. (Хотя, к сожалению, этот эффект уменьшается из-за необходимости проверки «мои операнды могут быть числовыми, и я не могу быть оператором отношений на данный момент», что я предоставляю, является недостатком). Ваш статус героя не изменился, несмотря на несогласие со мной :-)
Дэвид Баллок
3
После прочтения требований задачи, затем ваше решение, код имеет смысл и читабелен. Но как разработчик 4 года (все еще в школе), чтение этого кода, а затем попытка выяснить, какие «бизнес-требования» были на месте, было бы головной болью. Из этого кода я не сразу понимаю " fromи passwordоба должны быть нулевыми или оба не должны быть нулевыми". Возможно, это только я и моя неопытность, но я предпочитаю более удобочитаемое решение, как в ответе @ stig-hemmer, даже если оно стоит еще 3 строки кода. Я думаю, я просто не сразу получаю bool != bool - это не интуитивно понятно.
Крис Сирефице
2
@DavidBullock ^Оператор является побитовым оператором; это на самом деле не означает, что его операнды логические. Логический оператор XOR xor.
Brilliand
222

Лично я предпочитаю читабельность элегантному.

if (from != null && password == null) {
    throw new RuntimeException("-from given without -password");
}
if (from == null && password != null) {
    throw new RuntimeException("-password given without -from");
}
Стиг Хеммер
источник
52
+1 за лучшие сообщения. Это является важным, никто не любит handwaving «что - то пошло не так» -ошибка сообщения, так что никто не должен вызывать такие сообщения. Однако на практике следует предпочесть более конкретное исключение (в частности, IllegalArgumentExceptionвместо голого RuntimeException)
Marco13
6
@matt похоже, ваша критика не в коде, а в тексте исключений. Но я думаю, что заслуга этого ответа заключается в структуре операторов if, а не в содержании сообщений об ошибках. Это лучший ответ, потому что он улучшает функциональность; OP может легко заменить любые строки для текстов исключений.
Дэн Хендерсон
4
@matt Преимущество состоит в том, что становится возможным различить, какое из двух недопустимых состояний произошло, и настроить сообщение об ошибке. Таким образом, вместо того чтобы сказать «вы сделали одну из этих двух вещей неправильно», вы можете сказать «вы сделали это» или «вы сделали это» (а затем перейти к этапам разрешения, если это необходимо). Разрешение может быть одинаковым в обоих случаях, но никогда не стоит говорить «вы сделали одну из этих вещей неправильно». Источник: в среде, в которой я не смог предоставить эту функцию, я ответил на множество вопросов от пользователей, которые выполнили первое условие в моем тексте ошибки.
Дэн Хендерсон
4
@DanHenderson> никогда не стоит говорить "ты сделал одну из этих вещей неправильно". Я не согласен. Пароль / Имя пользователя, безопаснее просто сказать, что имя пользователя и пароль не совпадают.
матовый
2
@DanHenderson С точки зрения безопасности, может быть, лучше не делать различий между случаем, когда имя пользователя находится в каталоге, или нет, так как в противном случае злоумышленник может узнать действительные имена пользователей. Однако смешивание null / not null (в данном случае) всегда является ошибкой использования, и показ более подробного сообщения об ошибке не приводит к утечке больше информации, чем пользователь предоставил в первую очередь.
siegi
16

Поместите эту функциональность в метод с двумя аргументами с подписью:

void assertBothNullOrBothNotNull(Object a, Object b) throws RuntimeException

Это экономит место в интересующем вас фактическом методе и делает его более читабельным. Нет ничего плохого в слегка многословных именах методов, и нет ничего плохого в очень коротких методах.

Traubenfuchs
источник
14
Нет места для сохранения, ((from == null) != (password == null))что очень просто для понимания. Что-то не так с бесполезными методами.
edc65
3
Есть одна строка для оператора if, вторая строка для оператора throw и третья строка для закрывающей фигурной скобки: все заменены одной строкой. Сохраняется еще одна строка, если вы даете закрывающим скобкам новую строку!
Traubenfuchs
10
При чтении кода у вас есть одно имя метода, вы должны понимать, что такое жонглирование условиями.
Traubenfuchs
1
безусловно, +1, всегда предпочитайте абстрагироваться от подробностей реализации и операторов реализации с помощью описательных методов и имен переменных, которые делают INTENTION понятным. логика содержит ошибки. комментарии еще хуже. имя метода показывает намерение и изолирует логику, чтобы легче было находить и исправлять ошибки. это решение не требует, чтобы читатель знал или думал о каком-либо синтаксисе или логике вообще, вместо этого оно позволяет нам сосредоточиться на ТРЕБОВАНИЯХ ДЛЯ БИЗНЕСА (что действительно имеет значение)
sara
1
Вы столкнетесь с ненужными неприятностями, если захотите получить здесь описательное сообщение об исключении.
Хонза Брабек
11

Решение Java 8 будет использовать Objects.isNull(Object), предполагая статический импорт:

if (isNull(from) != isNull(password)) {
    throw ...;
}

Для Java <8 (или если вы не любите использовать Objects.isNull()), вы можете легко написать свой собственный isNull()метод.

Дидье Л
источник
6
Не нравится from == null != password == nullдержит все это в одном кадре стека, но Objects.isNull(Object)без необходимости толкает и выталкивает два кадра. Objects.isNull (Object) существует потому, что «Этот метод существует для использования в качестве предиката» (т. Е. В потоках).
Дэвид Баллок
5
Простой метод, подобный этому, обычно быстро внедряется JIT, поэтому влияние на производительность, скорее всего, незначительно. Мы действительно могли бы обсудить использование Objects.isNull()- вы можете написать свой собственный, если хотите, - но что касается читабельности, я думаю, что использование isNull()лучше. Кроме того вам нужны дополнительные скобки , чтобы сделать простое выражение компиляции: from == null != (password == null).
Дидье Л
2
Я согласен с JIT'ом (и порядком операций ... мне было лень). Тем не менее, (val == null)это очень большая возможность, предоставляемая для сравнения с нулем, и мне трудно преодолеть два больших толстых вызова метода, уставившихся мне в лицо, даже если метод довольно функциональный, встроенный и хорошо работающий. по имени. Это только я, хотя. Недавно я решил, что я немного странный.
Дэвид Баллок
4
честно, кого волнует один кадр стека? стек является проблемой, только если вы имеете дело с (возможно, бесконечной) рекурсией, или если у вас есть 1000-уровневое приложение с графом объектов размером с пустыню Сахара.
Сара
9

Вот общее решение для любого количества нулевых проверок

public static int nulls(Object... objs)
{
    int n = 0;
    for(Object obj : objs) if(obj == null) n++;
    return n;
}

public static void main (String[] args) throws java.lang.Exception
{
    String a = null;
    String b = "";
    String c = "Test";

    System.out.println (" "+nulls(a,b,c));
}

Пользы

// equivalent to (a==null & !(b==null|c==null) | .. | c==null & !(a==null|b==null))
if (nulls(a,b,c) == 1) { .. }

// equivalent to (a==null | b==null | c==null)
if (nulls(a,b,c) >= 1) { .. }

// equivalent to (a!=null | b!=null | c!=null)
if (nulls(a,b,c) < 3) { .. }

// equivalent to (a==null & b==null & c==null)
if (nulls(a,b,c) == 3) { .. }

// equivalent to (a!=null & b!=null & c!=null)
if (nulls(a,b,c) == 0) { .. }
Khaled.K
источник
2
Хороший подход, но ваш первый «эквивалентный» комментарий ужасно неправильный (за исключением орфографической ошибки, которая присутствует во всех комментариях)
Бен Фойгт
@BenVoigt Спасибо за уведомление, исправлено сейчас
Khaled.K
9

Поскольку вы хотите сделать что-то особенное (использовать значения по умолчанию), когда отсутствуют и отправитель, и пароль, обработайте это в первую очередь.
После этого у вас должен быть как отправитель, так и пароль для отправки электронной почты; бросить исключение, если либо отсутствует.

// use defaults if neither is provided
if ((from == null) && (password == null)) {
    from = DEFAULT_SENDER;
    password = DEFAULT_PASSWORD;
}

// we should have a sender and a password now
if (from == null) {
    throw new MissingSenderException();
}
if (password == null) {
    throw new MissingPasswordException();
}

Дополнительным преимуществом является то, что, если любое из ваших значений по умолчанию будет нулевым, это также будет обнаружено.


Сказав это, в общем, я думаю, что использование XOR должно быть допустимо, когда это тот оператор, который вам нужен. Это является частью языка, а не только какой - то трюк , который работает из - за загадочного компилятора жука.
У меня когда-то был коровник, который нашел троичного оператора слишком запутанным, чтобы использовать ...

SQB
источник
1
Проголосовал, потому что я сделал выборочный выбор одного из ваших ответов и не могу найти обещанную ссылку xkcd! тебе должно быть стыдно!
dhein
@ Zaibis В мою защиту, это просто хобби, а не работа на полный рабочий день. Но я посмотрю, смогу ли я найти один ...
SQB
8

Я хотел бы предложить другую альтернативу, которая заключается в том, как бы я на самом деле написал этот кусок кода:

if( from != null )
{
    if( password == null )
        error( "password required for " + from );
}
else
{
    if( password != null )
        warn( "the given password will not be used" );
}

Мне кажется, что это наиболее естественный способ выразить это состояние, которое облегчает понимание тем, кому, возможно, придется читать его в будущем. Это также позволяет вам выдавать более полезные диагностические сообщения и рассматривать ненужный пароль как менее серьезный, а также позволяет легко изменять, что весьма вероятно для такого состояния. Т.е. вы можете обнаружить, что указание пароля в качестве аргумента командной строки - не лучшая идея, и может потребоваться разрешить чтение пароля из стандартного ввода, если аргумент отсутствует. Или вы можете молча проигнорировать лишний аргумент пароля. Подобные изменения не потребуют от вас переписывания всего этого.

Кроме того, он выполняет только минимальное количество сравнений, поэтому он не дороже, чем более «элегантные» альтернативы. Хотя производительность здесь маловероятна, поскольку запуск нового процесса уже намного дороже, чем дополнительная проверка на ноль.

x4u
источник
Я комментирую несколько разговорно, так что я мог бы следовать этому еще с "// у нас есть ноль от". Краткость примера делает это действительно незначительным. Мне действительно нравится ясность логики и сокращение тестов, которые это представляет.
Nate
Это прекрасно работает, но вложение, если утверждения кажутся мне менее понятными и более загроможденными. Это просто личное мнение, поэтому я рад, что этот ответ здесь, как другой вариант
Кевин Уэллс,
Что касается элегантности, это, вероятно, один из наименее элегантных способов сделать это.
драмзи
7

Я думаю, что правильный способ справиться с этим - рассмотреть три ситуации: оба «from» и «password» предоставлены, ни один не предоставлен, оба из них предоставлены.

if(from != null && password != null){
    //use the provided values
} else if(from == null && password == null){
    //both values are null use the default values
} else{
   //throw an exception because the input is not correct.
}

Звучит так, будто исходный вопрос хочет прервать поток, если он введен неправильно, но тогда им придется повторить часть логики позже. Возможно, хороший бросок может быть следующим:

throw new IllegalArgumentException("form of " + form + 
    " cannot be used with a "
    + (password==null?"null":"not null") +  
    " password. Either provide a value for both, or no value for both"
);
матовый
источник
2
Этот код неверен не потому, что он не работает, а потому, что его очень сложно понять. Отладка такого кода, написанного кем-то другим, просто кошмар.
NO_NAME
2
@NO_NAME Я не понимаю, почему это трудно понять. ФП предоставил три случая: когда предоставлены и форма, и пароль, когда ни один не предоставлен, и смешанный случай, который должен вызвать исключение. Вы обращаетесь к условному среднему, чтобы проверить, не являются ли они оба нулевыми?
Мэтт
2
Я согласен с @NO_NAME, что в среднем регистре требуется слишком много времени для анализа. Если вы не проанализируете верхнюю строку, вы не получите, что нуль имеет какое-либо отношение к середине. В этом случае равенство from и пароля на самом деле является лишь побочным эффектом отсутствия нуля.
jimm101
1
@ jimm101 Да, я понял, что это проблема. Выигрыш в производительности будет минимальным / не обнаруживаемым для более подробного решения. Я не уверен, что проблема в этом. Я обновлю это.
матовый
2
@matt Может не быть выигрыша в производительности - неясно, что будет делать компилятор, и что чип будет извлекать из памяти в этом случае. Многие программистские циклы могут быть сожжены при оптимизации, которая на самом деле ничего не дает.
jimm101
6

Вот сравнительно простой способ, который не требует каких-либо длинных if. Однако он требует, чтобы вы были немного более многословны, но с другой стороны, вы можете использовать пользовательские исключения, которые я предложил, чтобы получить более значимое сообщение об ошибке.

private void validatePasswordExists(Parameters params) {
   if (!params.hasKey("password")){
      throw new PasswordMissingException("Password missing");
   }
}

private void validateFromExists(Parameters params) {
   if (!params.hasKey("from")){
      throw new FromEmailMissingException("From-email missing");
   }
}

private void validateParams(Parameters params) {

  if (params.hasKey("from") || params.hasKey("password")){
     validateFromExists(params);
     validatePasswordExists(params);
  }
}
Арнаб Датта
источник
1
Вы не должны использовать исключения вместо оператора потока управления. Помимо снижения производительности, более важно то, что он снижает читабельность вашего кода, потому что нарушает умственный поток.
phu
1
Нет, просто нет. Использование исключений повышает читабельность кода, поскольку избавляет от предложений if-else и делает поток кода более понятным. docs.oracle.com/javase/tutorial/essential/exceptions/…
Датта
1
Я думаю, что вы путаете что-то действительно исключительное с тем, что происходит регулярно и может считаться частью нормального исполнения. Документ Java использует примеры, которые являются исключительными, т.е. не хватает памяти при чтении файла; Обнаружение недействительных параметров не является исключением. «Не используйте исключения для нормального потока управления». - blogs.msdn.com/b/kcwalina/archive/2005/03/16/396787.aspx
An Phu
Ну, во-первых: если отсутствующие / правильные аргументы не являются исключительными, почему тогда существует исключение IllegalArgumentException? Второе: если вы действительно верите, что недействительные аргументы не являются исключительными, то и проверки их не должно быть. Другими словами, просто попробуйте выполнить действие, и если что-то пойдет не так, выведите исключение. Этот подход хорош, но здесь речь идет о снижении производительности, а не о предложенном мною подходе. Java-исключения не являются медленными при выдаче; это трассировка стека, для восстановления которой требуется время.
Арнаб Датта
Контекст является ключевым. В контексте OP (приложение sendmail) забывание имени пользователя и пароля является обычным явлением. В алгоритме наведения ракеты параметр с нулевой координатой должен выдавать исключение. Я не сказал, не проверять параметры. В контексте OP я сказал, что не используйте исключения для управления логикой приложения. Раскручивание стека - это тот самый хит, о котором я говорю. Используя ваш подход, в конечном итоге вы либо перехватываете исключение PasswordMissingException / FromEmailMissingException, т. Е. Раскручиваете или, что еще хуже, оставляете его необработанным. ,
phu
6

Кажется, никто не упомянул троичный оператор :

if (a==null? b!=null:b==null)

Хорошо работает для проверки этого конкретного условия, но не обобщает хорошо за две переменные.

gbronner
источник
1
Выглядит хорошо, но труднее понять , чем ^, !=с двумя BOOLS или два ifс
coolguy
@coolguy Я предполагаю, что троичный оператор встречается гораздо чаще, чем оператор XOR (не говоря уже о «ментальном удивлении» каждый раз, когда вы видите XOR в коде, который не выполняет битовые операции), и эта формулировка стремится избежать сокращения и вставьте ошибки, которые мешают двойному, если.
gbronner
Не рекомендуется. Это не будет различать (a! = Null && b == null) и (a == null && b! = Null). Если вы собираетесь использовать троичный оператор:a == null ? (b == null? "both null" : "a null while b is not") : (b ==null? "b null while a is not")
Арнаб Датта
5

Как я понимаю ваши намерения, нет необходимости всегда проверять обе исключительные значения, а проверять, passwordявляется ли значение null, если и только если fromоно не равно NULL. Вы можете игнорировать данный passwordаргумент и использовать свой собственный параметр по умолчанию, если значение fromравно нулю.

Написано псевдо должно быть так:

if (from == null) { // form is null, ignore given password here
    // use your own defaults
} else if (password == null) { // form is given but password is not
    // throw exception
} else { // both arguments are given
    // use given arguments
}
JordiVilaplana
источник
4
Единственная проблема с этим состоит в том, что, когда пользователь поставляет passwordбез предоставления from, тогда он, возможно, намеревался переопределить пароль, но сохранить учетную запись по умолчанию. Если пользователь делает это, это неверная инструкция, и ему следует сообщить об этом. Программа не должна работать так, как если бы ввод был верным, и попытаться использовать учетную запись по умолчанию с паролем, который пользователь не указал. Я клянусь в таких программах.
Дэвид Баллок
4

Я удивлен, что никто не упомянул простое решение создания fromи passwordполей класса и передачи ссылки на экземпляр этого класса:

class Account {
    final String name, password;
    Account(String name, String password) {
        this.name = Objects.requireNonNull(name, "name");
        this.password = Objects.requireNonNull(password, "password");
    }
}

// the code that requires an account
Account from;
// do stuff

Здесь fromможет быть ноль или ненулевое значение, и если оно ненулевое, оба его поля имеют ненулевые значения.

Одним из преимуществ этого подхода является то, что ошибка, связанная с тем, что одно поле, но не другое поле, становится пустым, возникает при первоначальном получении учетной записи, а не при запуске кода, использующего эту учетную запись. К тому времени, когда код, использующий учетную запись, будет выполнен, данные станут недействительными.

Другое преимущество этого подхода более читабельно, так как предоставляет больше семантической информации. Кроме того, вполне вероятно, что вам требуется имя и пароль вместе в других местах, поэтому стоимость определения дополнительного класса амортизируется в течение нескольких раз.

Восстановить Монику
источник
Он не работает должным образом, поскольку new Account(null, null)выдает NPE, даже если учетная запись с нулевым именем и паролем все еще действительна.
HieuHT
Идея состоит в том, чтобы передать значение NULL, если имя и пароль имеют значение NULL, вне зависимости от того, существует ли в реальном мире учетная запись.
Восстановить Монику
То, что я получаю от OP, это то есть, либо оба ненулевые, либо оба нулевые. Мне просто интересно, как вы можете создать объект учетной записи с именем и паролем null?
HieuHT
@HieuHT Извините, мой последний комментарий был неясен. Если имя и пароль пустые, вы должны использовать nullвместо Account. Вы не перейдете nullк Accountконструктору.
Восстановить Монику