구문 나쁜 관행을 만족시키기 위해 return 문이 있습니까?

구문 나쁜 관행을 만족시키기 위해 return 문이 있습니까?

---

다음 코드를 고려하십시오.

public Object getClone(Cloneable a) throws TotallyFooException {

    if (a == null) {
        throw new TotallyFooException();
    }
    else {
        try {
            return a.clone();
        } catch (CloneNotSupportedException e) {
            e.printStackTrace();
        }
    }
    //cant be reached, in for syntax
    return null;
}

는 return null;예외가 잡힐 수 있기 때문에 그러나 우리는 이미 널 (null)이 있다면 검사 (우리는 우리가 복제 지원을 호출하는 클래스를 알고 있다고 가정 할 수 있습니다) 우리가 try 문이 실패하지 않습니다 알 수 있도록하기 때문에 이러한 경우에 필요하다.

구문을 충족하고 컴파일 오류를 피하기 위해 끝에 추가 return 문을 넣는 것이 나쁜 습관입니까 (주석에 도달하지 않을 것임을 설명하는 주석 포함), 아니면 이와 같은 코드를 작성하는 더 좋은 방법이 있습니까? return 문이 필요하지 않습니까?

---

추가 return 문이없는 더 명확한 방법은 다음과 같습니다. 나도 잡지 CloneNotSupportedException못하지만 발신자에게 전달합니다.

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

처음에 가지고있는 것보다 더 간단한 구문으로 끝나도록 순서를 조정하는 것은 거의 항상 가능합니다.

---

확실히 도달 할 수 있습니다. catch절 에서 stacktrace 만 인쇄한다는 점에 유의하십시오 .

시나리오에서 a != null거기 것이다 예외가 될 수는는 return null 것이다 도달 할 수. 해당 문을 제거하고 throw new TotallyFooException();.

일반적으로 ^* , if null가 메서드 의 유효한 결과 (즉, 사용자가 예상 하고 의미하는 바가 있음) 인 경우 "데이터를 찾을 수 없음"또는 예외 발생에 대한 신호로 반환하는 것은 좋은 생각 이 아닙니다 . 그렇지 않으면 반환하지 않아야하는 이유를 알 수 없습니다 null.

예를 들어 Scanner#ioException방법을 살펴보십시오 .

IOException이 Scanner 기본이되는 Readable이 던진 마지막을 리턴합니다 . 이러한 예외가 없으면 이 메서드 는 null을 반환 합니다 .

이 경우 반환 된 값 null은 분명한 의미를 가지고 있습니다. 메소드를 사용하면 메소드 null가 무언가를하려고해서 실패한 것이 아니라 그러한 예외가 없었기 때문에 얻은 것임을 확신 할 수 있습니다 .

^* 때로는 null의미가 모호한 경우에도 반환하고 싶을 때가 있습니다 . 예를 들어 HashMap#get:

반환 값이 null 이라고해서 반드시 맵에 키에 대한 매핑이 포함되어 있지 않음을 나타내는 것은 아닙니다. 지도가 명시 적으로 키를 null로 매핑 할 수도 있습니다 . containsKey작업은이 두 경우를 구별 할 수 있습니다.

이 경우 값 을 찾아서 반환했거나 해시 맵에 요청 된 키가 포함되어 있지 않음을 null나타낼 수 있습니다 . null

---

구문을 만족시키고 컴파일 오류를 피하기 위해 끝에 추가 return 문을 넣는 것이 나쁜 습관입니까? (설명에 도달하지 않을 것임을 설명하는 주석 포함)

return null도달 할 수없는 분기의 종점에는 나쁜 습관 이라고 생각 합니다. 던져 더 RuntimeException( AssertionError그 라인 뭔가 아주 잘못 간에 도착하고 응용 프로그램이 알 수없는 상태에로서 또한 허용 될 것이다). 개발자가 무언가를 놓 쳤기 때문입니다 (객체는 null이 아니고 복제가 불가능할 수 있음).

VM보다 실수 할 가능성이 더 높기 때문에 InternalError코드에 도달 할 수 없는지 (예 : a 이후 System.exit()) 확실하지 않으면 사용 하지 않을 것입니다.

TotallyFooException"연결할 수없는 줄"에 도달하는 것이 해당 예외를 던지는 다른 곳과 동일한 의미 인 경우 에만 사용자 지정 예외 (예 :)를 사용합니다.

---

당신 CloneNotSupportedException은 당신의 코드가 그것을 처리 할 수 ​​있다는 것을 의미합니다. 그러나 그것을 잡은 후에는 함수의 끝에 도달했을 때 무엇을해야할지 전혀 알지 못합니다. 이것은 그것을 처리 할 수 ​​없다는 것을 의미합니다. 따라서이 경우 코드 냄새라는 것이 맞습니다. 제 생각에는 CloneNotSupportedException.

---

Objects.requireNonNull()매개 변수 a가 null이 아닌지 확인 하는 데 사용 하고 싶습니다 . 따라서 코드를 읽을 때 매개 변수가 null이 아니어야한다는 것이 분명합니다.

그리고 피할 검사 예외에 내가 던져 다시 것이다 CloneNotSupportedExceptionA와 RuntimeException.

둘 다 왜 이런 일이 일어나지 않아야하는지 또는 그렇지 않은지 의도와 함께 멋진 텍스트를 추가 할 수 있습니다.

public Object getClone(Object a) {

    Objects.requireNonNull(a);

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

}

---

이런 상황에서

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) ; } }
}

Interestingly you say that the "try statement will never fail", but you still took the trouble to write a statement e.printStackTrace(); that you claim will never be executed. Why?

Perhaps your belief is not that firmly held. That is good (in my opinion), since your belief is not based on the code you wrote, but rather on the expectation that your client will not violate the precondition. Better to program public methods defensively.

By the way, your code won't compile for me. You can't call a.clone() even if the type of a is Cloneable. At least Eclipse's compiler says so. Expression a.clone() gives error

The method clone() is undefined for the type Cloneable

What I would do for your specific case is

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

Where PubliclyCloneable is defined by

interface PubliclyCloneable {
    public Object clone() ;
}

Or, if you absolutely need the parameter type to be Cloneable, the following at least compiles.

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) ; } }
}

---

The examples above are valid and very Java. However, here's how I would address the OP's question on how to handle that return:

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

There's no benefit for checking a to see if it is null. It's going to NPE. Printing a stack trace is also not helpful. The stack trace is the same regardless of where it is handled.

There is no benefit to junking up the code with unhelpful null tests and unhelpful exception handling. By removing the junk, the return issue is moot.

(Note that the OP included a bug in the exception handling; this is why the return was needed. The OP would not have gotten wrong the method I propose.)

---

Is having a return statement just to satisfy syntax bad practice?

As others have mentioned, in your case this does not actually apply.

To answer the question, though, Lint type programs sure haven't figured it out! I have seen two different ones fight it out over this in a switch statement.

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

One complained that not having the break was a coding standard violation. The other complained that having it was one because it was unreachable code.

I noticed this because two different programmers kept re-checking the code in with the break added then removed then added then removed, depending on which code analyzer they ran that day.

If you end up in this situation, pick one and comment the anomaly, which is the good form you showed yourself. That's the best and most important takeaway.

---

It isn't 'just to satisfy syntax'. It is a semantic requirement of the language that every code path leads to a return or a throw. This code doesn't comply. If the exception is caught a following return is required.

No 'bad practice' about it, or about satisfying the compiler in general.

In any case, whether syntax or semantic, you don't have any choice about it.

---

I would rewrite this to have the return at the end. Pseudocode:

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

return b

---

No one mentioned this yet so here goes:

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;

}

I dislike return inside try blocks for that very reason.

---

The return null; is necessary since an exception may be caught, however in such a case since we already checked if it was null (and lets assume we know the class we are calling supports cloning) so we know the try statement will never fail.

If you know details about the inputs involved in a way where you know the try statement can never fail, what is the point of having it? Avoid the try if you know for sure things are always going to succeed (though it is rare that you can be absolutely sure for the whole lifetime of your codebase).

In any case, the compiler unfortunately isn't a mind reader. It sees the function and its inputs, and given the information it has, it needs that return statement at the bottom as you have it.

Is it bad practice to put in the extra return statement at the end just to satisfy the syntax and avoid compile errors (with a comment explaining it will not be reached), or is there a better way to code something like this so that the extra return statement is unnecessary?

Quite the opposite, I'd suggest it's good practice to avoid any compiler warnings, e.g., even if that costs another line of code. Don't worry too much about line count here. Establish the reliability of the function through testing and then move on. Just pretending you could omit the return statement, imagine coming back to that code a year later and then try to decide if that return statement at the bottom is going to cause more confusion than some comment detailing the minutia of why it was omitted because of assumptions you can make about the input parameters. Most likely the return statement is going to be easier to deal with.

That said, specifically about this part:

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

I think there's something slightly odd with the exception-handling mindset here. You generally want to swallow exceptions at a site where you have something meaningful you can do in response.

You can think of try/catch as a transaction mechanism. try making these changes, if they fail and we branch into the catch block, do this (whatever is in the catch block) in response as part of the rollback and recovery process.

In this case, merely printing a stacktrace and then being forced to return null isn't exactly a transaction/recovery mindset. The code transfers the error-handling responsibility to all the code calling getClone to manually check for failures. You might prefer to catch the CloneNotSupportedException and translate it into another, more meaningful form of exception and throw that, but you don't want to simply swallow the exception and return a null in this case since this is not like a transaction-recovery site.

You'll end up leaking the responsibilities to the callers to manually check and deal with failure that way, when throwing an exception would avoid this.

It's like if you load a file, that's the high-level transaction. You might have a try/catch there. During the process of trying to load a file, you might clone objects. If there's a failure anywhere in this high-level operation (loading the file), you typically want to throw exceptions all the way back to this top-level transaction try/catch block so that you can gracefully recover from a failure in loading a file (whether it's due to an error in cloning or anything else). So we generally don't want to just swallow up an exception in some granular place like this and then return a null, e.g., since that would defeat a lot of the value and purpose of exceptions. Instead we want to propagate exceptions all the way back to a site where we can meaningfully deal with it.

---

Your example is not ideal to illustrate your question as stated in the last paragraph:

Is it bad practice to put in the extra return statement at the end just to satisfy the syntax and avoid compile errors (with a comment explaining it will not be reached), or is there a better way to code something like this so that the extra return statement is unnecessary?

A better example would be the implementation of clone itself:

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

Here the catch clause should never be entered. Still the syntax either requires to throw something or return a value. Since returning something does not make sense, an InternalError is used to indicate a severe VM condition.

참고URL : https://stackoverflow.com/questions/33804394/is-having-a-return-statement-just-to-satisfy-syntax-bad-practice