Булево возвращение set.add (), если условно?

Оператор add класса set возвращает логическое значение, которое имеет значение true, если элемент (который должен быть добавлен) еще не существует, и false в противном случае. Пишет

if (set.add(entry)) {
    //do some more stuff
}

считается хорошим стилем с точки зрения написания чистого кода? Мне интересно, так как вы делаете две вещи одновременно. 1) добавление элемента и 2) проверка существования элемента.

Да, это так.

Обычный момент, когда операция возвращает логическое значение, заключается в том, что вы можете использовать его для принятия решений, то есть внутри ifконструкции. Единственный способ реализовать этот потенциал - это сохранить возвращаемое значение в переменной, а затем сразу же повторно использовать его в переменной if, что просто глупо и, безусловно, ни в коем случае не предпочтительнее записи if(operation()) { ... }. Так что просто продолжайте и сделайте это, мы не будем судить вас (за это).

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

Я бы предпочел следующее:

boolean added = set.add(entry);

if (added) {
    //do some more stuff
}

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

Если true означает успех, то это хороший, понятный код.

Существует широко распространенное соглашение, что функция или метод возвращает true (или что-то, что оценивается как true) в случае успеха. Пока ваш код следует этому, я думаю, что помещать метод в условное выражение - это нормально.

Код, подобный этому, излишне загроможден, на мой взгляд:

boolean frobulate_succeeded = thing.frobulate();

if (frobulate_succeeded) {
    ...
}

Такое чувство, что ты повторяешься.

Однако вопрос о значении возвращаемого значения неоднозначен. Вы говорите «логическое значение, которое указывает, существовал ли добавленный элемент», что может означать, что true означает, что элемент существовал (а добавление не произошло). Если это так, я бы в идеале изменил поведение метода при возврате, чтобы оно было более привычным. Если это невозможно, я бы добавил дополнительную промежуточную переменную, которая позволит вам четко обозначить возвращаемый результат в вашем коде (как это предлагают другие).

Я бы сказал, что это очень C-like. Большую часть времени я бы предпочел иметь переменную с описательным именем для результата мутации, а в ifусловии не было бы никакой мутации .

Компилятор исключит эту переменную, если она будет немедленно использована повторно. Человеку будет легче читать источник; для меня это важнее.

Если кому-то придется расширить условие, добавив условие and/ orк условию if, .add()в некоторых случаях он может не вызывать из-за оценки короткого замыкания. Если короткое замыкание не предусмотрено специально, это может привести к ошибке.

Кажется, ваш код нарушает разделение командных запросов . Это обсуждается в книге « Чистый код» и в видео « Структура функций» . Так что с точки зрения чистого кода я думаю, что это не считается хорошим стилем.

«Чистый код прост и понятен. Чистый код читается как хорошо написанная проза. Чистый код никогда не затеняет замысел дизайнера, а полон четких абстракций и простых линий контроля.

- Грэди Буч, автор «Объектно-ориентированного анализа и проектирования с приложениями»

Для меня цель вашего кода неясна. Выполняется ли if оба при успешном добавлении записи или когда она уже существует? Что add()возвращает? пункт? Код ошибки?