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

Ответ 1

Более понятный способ без дополнительного оператора возврата выглядит следующим образом. Я бы тоже не поймал CloneNotSupportedException, но позвольте ему перейти к вызывающему.

if (a != null) {
    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        e.printStackTrace();
    }
}
throw new TotallyFooException();

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

Ответ 2

Это определенно может быть достигнуто. Обратите внимание, что вы только печатаете stacktrace в предложении catch.

В сценарии, где a != null и будет исключение, будет достигнуто return null . Вы можете удалить этот оператор и заменить его на throw new TotallyFooException();.

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

Возьмем, например, метод Scanner#ioException:

Возвращает IOException, последний раз брошенный этим сканером, лежащим в основе Readable. Этот метод возвращает null, если такое исключение не существует.

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

^* Обратите внимание, что иногда вы хотите вернуть null, даже если значение неоднозначно. Например, HashMap#get:

Возвращаемое значение null не обязательно указывает, что карта не содержит отображения для ключа; также возможно, что карта явно отображает ключ в null. Операция containsKey может использоваться для различения этих двух случаев.

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

Ответ 3

Неправильная практика заключается в том, чтобы вставить дополнительный оператор возврата в конце, чтобы удовлетворить синтаксис и избежать ошибок компиляции (с комментарием, объясняющим, что он не будет достигнут)

Я думаю, что return null - это плохая практика для конца недостижимой ветки. Лучше бросить RuntimeException (AssertionError также будет приемлемым), чтобы добраться до этой линии, что-то пошло не так, и приложение находится в неизвестном состоянии. Чаще всего это (например, выше), потому что разработчик что-то пропустил (объекты могут быть не нулевыми и не клонируемыми).

Я бы не использовал InternalError, если я не очень уверен, что код недоступен (например, после System.exit()), поскольку более вероятно, что я ошибаюсь, чем VM.

Я бы использовал только настраиваемое исключение (например, TotallyFooException), если попадание в эту "недостижимую строку" означает то же самое, что и в другом месте, которое вы выбрали для этого исключения.

Ответ 4

Вы поймали CloneNotSupportedException, что означает, что ваш код может справиться с этим. Но после того, как вы поймаете это, вы не понимаете, что делать, когда вы достигаете конца функции, что подразумевает, что вы не можете справиться с этим. Поэтому вы правы, что в этом случае это запах кода, и, на мой взгляд, вы не должны были поймать CloneNotSupportedException.

Ответ 5

Я бы предпочел использовать Objects.requireNonNull(), чтобы проверить, не является ли параметр a непустым. Поэтому ясно, когда вы читаете код, что параметр не должен быть нулевым.

И чтобы избежать отмеченных Исключений, я бы повторно выбрал CloneNotSupportedException как RuntimeException.

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

public Object getClone(Object a) {

    Objects.requireNonNull(a);

    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        throw new IllegalArgumentException(e);
    }

}

Ответ 6

В такой ситуации я бы написал

public Object getClone(SomeInterface a) throws TotallyFooException {
    // Precondition: "a" should be null or should have a someMethod method that
    // does not throw a SomeException.
    if (a == null) {
        throw new TotallyFooException() ; }
    else {
        try {
            return a.someMethod(); }
        catch (SomeException e) {
            throw new IllegalArgumentException(e) ; } }
}

Интересно, что вы говорите, что "оператор try никогда не будет терпеть неудачу", но вы по-прежнему не могли написать выражение e.printStackTrace();, которое, как вы утверждают, никогда не будет выполнено. Почему?

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

Кстати, ваш код не будет компилироваться для меня. Вы не можете вызвать a.clone(), даже если тип a равен Cloneable. По крайней мере, компилятор Eclipse говорит об этом. Выражение a.clone() дает ошибку

Метод clone() - undefined для типа Cloneable

Что бы я сделал для вашего конкретного случая,

public Object getClone(PubliclyCloneable a) throws TotallyFooException {
    if (a == null) {
        throw new TotallyFooException(); }
    else {
        return a.clone(); }
}

Где PubliclyCloneable определяется

interface PubliclyCloneable {
    public Object clone() ;
}

Или, если вам абсолютно необходим тип параметра Cloneable, по крайней мере компилируется следующее.

public static Object getClone(Cloneable a) throws TotallyFooException {
//  Precondition: "a" should be null or point to an object that can be cloned without
// throwing any checked exception.
    if (a == null) {
        throw new TotallyFooException(); }
    else {
        try {
            return a.getClass().getMethod("clone").invoke(a) ; }
        catch( IllegalAccessException e ) {
            throw new AssertionError(null, e) ; }
        catch( InvocationTargetException e ) {
            Throwable t = e.getTargetException() ;
            if( t instanceof Error ) {
                // Unchecked exceptions are bubbled
                throw (Error) t ; }
            else if( t instanceof RuntimeException ) {
                // Unchecked exceptions are bubbled
                throw (RuntimeException) t ; }
            else {
                // Checked exceptions indicate a precondition violation.
                throw new IllegalArgumentException(t) ; } }
        catch( NoSuchMethodException e ) {
            throw new AssertionError(null, e) ; } }
}

Ответ 7

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

public Object getClone(Cloneable a) throws CloneNotSupportedException {
    return a.clone();
}

Нет никакой пользы для проверки a, чтобы увидеть, является ли оно нулевым. Это будет NPE. Печать трассировки стека также не помогает. Трассировка стека одинакова независимо от того, где она обрабатывается.

Нет никакой пользы для разрастания кода с бесполезными нулевыми тестами и бесполезной обработкой исключений. Удаляя мусор, возвращение вопрос спорный вопрос.

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

Ответ 8

Имеет оператор return только для того, чтобы удовлетворить синтаксическую плохую практику?

Как уже упоминалось, в вашем случае это фактически не применяется.

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

    switch (var)
   {
     case A:
       break;
     default:
       return;
       break;    // Unreachable code.  Coding standard violation?
   }

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

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

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

Ответ 9

Это не "просто удовлетворить синтаксис". Это семантическое требование языка, что каждый путь кода приводит к возврату или броску. Этот код не соответствует. Если исключение поймано, требуется следующее возвращение.

Нет "плохой практики" об этом или об удовлетворении компилятора вообще.

В любом случае, будь то синтаксис или семантика, у вас нет выбора.

Ответ 10

Я бы переписал это, чтобы вернуться в конце. Псевдокод:

if a == null throw ...
// else not needed, if this is reached, a is not null
Object b
try {
  b = a.clone
}
catch ...

return b

Ответ 11

Никто не упомянул об этом, так вот:

public static final Object ERROR_OBJECT = ...

//...

public Object getClone(Cloneable a) throws TotallyFooException {
Object ret;

if (a == null) 
    throw new TotallyFooException();

//no need for else here
try {
    ret = a.clone();
} catch (CloneNotSupportedException e) {
    e.printStackTrace();
    //something went wrong! ERROR_OBJECT could also be null
    ret = ERROR_OBJECT; 
}

return ret;

}

Мне не нравится return внутри try блоков по этой причине.

Ответ 12

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

Если вы знаете подробности о входах, участвующих в том, как вы знаете, что оператор try никогда не может потерпеть неудачу, в чем его смысл? Избегайте try, если вы точно знаете, что все всегда будет успешным (хотя редко можно быть абсолютно уверенным на протяжении всей вашей кодовой базы).

В любом случае компилятор, к сожалению, не является читателем разума. Он видит функцию и ее входные данные и получает информацию, которая у нее есть, ей нужен оператор return внизу, как и у вас.

Неправильно ли в конце добавить оператор возврата просто чтобы удовлетворить синтаксис и избежать ошибок компиляции (с комментарием объяснение этого не будет достигнуто), или есть лучший способ кодирования что-то вроде этого, так что лишний оператор возврата не нужен?

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

Тем самым, в частности, об этой части:

try {
    return a.clone();
} catch (CloneNotSupportedException e) {
   e.printStackTrace();
}
...
//cant be reached, in for syntax
return null;

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

Вы можете думать о try/catch как о механизме транзакции. try, делая эти изменения, если они терпят неудачу, и мы входим в блок catch, сделаем это (все, что находится в блоке catch) в ответ как часть процесса отката и восстановления.

В этом случае просто печать stacktrace и принудительное возвращение null не является точно мышлением транзакции/восстановления. Код передает ответственность за обработку ошибок ко всему коду, вызывающему getClone, чтобы вручную проверить наличие сбоев. Возможно, вы захотите поймать CloneNotSupportedException и перевести его в другую, более значимую форму исключения и выбросить, но вы не хотите просто проглатывать исключение и возвращать null в этом случае, поскольку это не похоже на транзакцию, сайт восстановления.

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

Как будто вы загружаете файл, это транзакция высокого уровня. У вас может быть try/catch. Во время процесса trying для загрузки файла вы можете клонировать объекты. Если в этой высокоуровневой операции (загрузка файла) произошел сбой, вы обычно хотите исключить исключения на этот транзакционный блок верхнего уровня try/catch, чтобы вы могли изящно восстановить из-за сбоя при загрузке файла (будь то из-за ошибки в клонировании или чего-либо еще). Таким образом, мы обычно не хотим просто проглатывать исключение в каком-то гранулированном месте, таком как это, а затем возвращать нуль, например, так как это приведет к поражению большого количества и цели исключений. Вместо этого мы хотим распространять исключения на обратном пути к сайту, на котором мы можем с этим справиться.

Ответ 13

Ваш пример не идеален для иллюстрации вашего вопроса, как указано в последнем абзаце:

Неправильно ли в конце добавить оператор возврата просто чтобы удовлетворить синтаксис и избежать ошибок компиляции (с комментарием объяснение этого не будет достигнуто), или есть лучший способ кодирования что-то вроде этого, так что лишний оператор возврата не нужен?

Лучшим примером может быть реализация самого клона:

 public class A implements Cloneable {
      public Object clone() {
           try {
               return super.clone() ;
           } catch (CloneNotSupportedException e) {
               throw new InternalError(e) ; // vm bug.
           }
      }
 }

Здесь предложение catch не должно вводиться. Тем не менее синтаксис требует либо бросить что-то, либо вернуть значение. Так как возвращение чего-то не имеет смысла, InternalError используется для обозначения тяжелого состояния VM.