【问题标题】:Is returning an exception an anti-pattern?返回异常是反模式吗?
【发布时间】:2018-09-22 14:32:06
【问题描述】:

我有两个简单的方法:

public void proceedWhenError() {
   Throwable exception = serviceUp();

   if (exception == null) {
      // do stuff
   } else {
      logger.debug("Exception happened, but it's alright.", exception)
      // do stuff
   }
}

public void doNotProceedWhenError() {
   Throwable exception = serviceUp();

   if (exception == null) {
      // do stuff
   } else {
      // do stuff
      throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
   }
}

第三种方法是私有辅助方法:

private Throwable serviceUp() {
    try {
        service.connect();
        return null;
    catch(Exception e) {
       return e;
    }
}

我们与我的一位同事就此处使用的模式进行了简短的交谈:

serviceUp() 方法返回一个Exception(或Throwable)对象

第一个意见:

使用异常来控制工作流是一种反模式,我们应该只返回布尔值 serviceUp() 而不是 Exception 对象本身。论据是 使用异常来控制工作流是一种反模式。

第二种意见:

没关系,因为我们需要在后面处理对象 两种第一种方法不同以及是否返回异常对象 或布尔值根本不会改变工作流程

您认为 1) 或 2) 正确吗?尤其是,为什么? 请注意,问题仅与 serviceUp() 方法及其返回类型有关 - booleanException 对象。

注意:我不是在质疑是否使用 Throwable 或 Exception 对象。

【问题讨论】:

  • 为什么要返回异常而不是抛出异常?
  • @Turing85 这就是整个问题的意义所在。 :)
  • @Turing85 ?你的意思是serviceUp()?在proceedWhenError() 我们不想抛出异常。
  • @lexicore:不,问题似乎没有提到抛出异常作为选项。
  • 我认为问为什么它返回异常而不是允许它被抛出的关键在于捕获它,返回它,检查 null 与允许它被抛出是一样的(不捕获它)并在调用函数时在上面的级别捕获它,除非它需要额外的步骤。换句话说,对 serviceUp() 的任何调用都可以替换为对 service.connect() 的调用,而“if”则替换为 catch。

标签: java exception error-handling exception-handling anti-patterns


【解决方案1】:

只有在非异常情况下抛出异常时才使用异常来引导流程是一种反模式*。例如,通过在到达集合末尾时抛出异常来结束循环是一种反模式。

另一方面,用实际异常控制流程是异常的一个很好的应用。如果您的方法遇到无法处理的异常情况,它应该抛出异常,从而将调用者中的流程重定向到异常处理程序块。

从方法中返回一个“裸”Exception 对象,而不是抛出它,当然是违反直觉的。如果您需要将操作的结果传达给调用者,更好的方法是使用包装所有相关信息的状态对象,包括异常:

public class CallStatus {
    private final Exception serviceException;
    private final boolean isSuccess;
    public static final CallStatus SUCCESS = new CallStatus(null, true);
    private CallStatus(Exception e, boolean s) {
        serviceException = e;
        isSuccess = s;
    }
    public boolean isSuccess() { return isSuccess; }
    public Exception getServiceException() { return serviceException; }
    public static CallStatus error(Exception e) {
        return new CallStatus(e, false);
    }
}

现在调用者将收到来自serviceUpCallStatus

CallStatus status = serviceUp();
if (status.isSuccess()) {
    ... // Do something
} else {
    ... // Do something else
    Exception ex = status.getException();
}

请注意,构造函数是私有的,因此serviceUp 将返回CallStatus.SUCCESS 或调用CallStatus.error(myException)

* 什么是例外,什么不是例外,很大程度上取决于上下文。例如,非数字数据会导致ScannernextInt 出现异常,因为它认为此类数据无效。但是,相同的确切数据不会导致hasNextInt 方法中的异常,因为它是完全有效的。

【讨论】:

    【解决方案2】:

    第二意见(“没关系”)不成立。代码不好,因为返回异常而不是抛出它们并不是真正地道。

    我也不认同第一种观点(“使用异常来控制工作流程是反模式”)。 service.connect() 正在抛出异常,您必须对此异常做出反应 - 所以这 有效的流控制。返回 boolean 或其他状态对象并处理它而不是处理异常 - 并认为它不是基于异常的控制流是天真的。另一个缺点是,如果您决定重新抛出异常(包含在 IllegalArgumentException 或其他任何内容中),您将不再拥有原始异常。当您尝试分析实际发生​​的事情时,这非常糟糕。

    所以我会做经典:

    • serviceUp 中抛出异常。
    • 在调用serviceUp的方法中:
      • try-catch,如果您想继续处理异常,请记录调试并吞下异常。
      • try-catch 并重新抛出包含在另一个异常中的异常,以提供有关发生情况的更多信息。或者,如果您无法添加任何实质性内容,则让原始异常通过 throws 传播。

    最重要的是不要丢失原始异常。

    【讨论】:

    • 我比接受的答案更喜欢这个。这很简单。让proceedWhenError 捕获异常并记录它。 serviceUp 甚至不需要 try/catch。
    • @dwilliss 而且 doNotProceedWhenError 也不会,除非你真的需要将异常包装在另一个异常中。
    • @Zeus 真的。虽然我喜欢这样做,但记录的主要错误会显示“无法连接到服务 XYZ”之类的内容,并带有实际错误的 innerException。因为实际的错误通常是模糊的,比如“连接被拒绝”,它不会告诉你 what 连接被拒绝。任何可以帮助支持团队的东西,这样他们就不必打电话询问这意味着什么。
    • @dwilliss 一般来说,我倾向于同意你的观点。
    【解决方案3】:

    两者都是错误的

    没关系,因为我们需要在第一个方法中以不同方式处理对象,并且返回异常对象或布尔值根本不会改变工作流程

    这不好。异常的概念意味着它们被抛出,嗯,例外情况。他们应该在他们将被处理的地方被抓住(或者至少在一些本地清理/记录/等之后重新抛出)。它们不应该像这样(即在“域”代码中)传递。

    人们会感到困惑。真正的错误很容易让它蔓延——例如,如果有一些Exception 来自其他来源而不是这里的网络;一个你没有预料到的,那真的很糟糕(比如你由一些编程错误创建的一些越界异常)?

    新手程序员会无休止地感到困惑,和/或将这种反模式复制到不属于它的地方。

    比如最近一个同事实现了一个比较复杂的接口(比如机器对机器的接口,不是Javainterface),他也做了类似的异常处理;将异常转换为静默忽略的变化(以一些日志消息为模)。不用说,任何他实际上没有预料到的例外以最糟糕的方式打破了整个混乱;与fail fast相反。

    使用异常来控制工作流是一种反模式,我们应该只从 serviceUp() 返回布尔值,而不是异常对象本身。争论是使用异常来控制工作流是一种反模式。

    异常肯定会在它们经常中止工作流或重定向到向用户显示的错误消息方面控制工作流。由于异常,绝对有可能使某些代码“禁用”;也就是说,除了顶级控制之外,肯定允许在其他地方处理异常。

    但是返回异常确实是一种反模式;没有人想到,这很奇怪,它会导致虚假错误(很容易忽略返回值)等等。

    因此,对于您的serviceUp(),要么将其设为void - 它要么在 99% 的时间内有效,要么引发异常;或者让它成为真的boolean,因为你完全接受它会在某个地方失败。如果您确实需要传递错误消息,请将其作为String 进行处理,或者将其保存在不碍事的地方或其他地方,但不要将其用作返回值,尤其是如果您打算使用throw 它稍后再来。

    简单的标准解决方案

    此解决方案更短(更少的行、更少的变量、更少的if)、更简单、符合标准,并且完全符合您的要求。易于维护,易于理解。

    public void proceedWhenError() {
       try {
          serviceUp();
          // do stuff (only when no exception)
       }
       catch (Exception exception) {
          logger.debug("Exception happened, but it's alright.", exception)
          // do stuff (only when exception)
       }
    }
    
    public void doNotProceedWhenError() {
       try {
          serviceUp();
          // do stuff (only when no exception)
       }
       catch (Exception exception) {
          // do stuff (only when exception)
          throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
       }
    }
    
    private void serviceUp() {
        service.connect();
    }
    

    【讨论】:

    • 您提出的解决方案是与“快速失败”相反的解决方案,丢弃“[您实际上没有预料到的任何异常”,从而隐藏这些问题并阻止它们得到修复。
    • @BenVoigt,该示例是对原始代码的简单直接重写,展示了如何在不处理 Exception 对象或执行 if 子句的情况下实现与他所做的完全相同的操作。
    【解决方案4】:

    我会返回一个ServiceState,例如,RUNNINGWAITINGCLOSED。该方法将被命名为getServiceState

    enum ServiceState { RUNNING, WAITING, CLOSED; }
    

    我从未见过由于执行而返回异常的方法。对我来说,当一个方法返回值时,这意味着该方法完成了它的工作没有问题。我不想检索结果并将其解析为包含任何错误。结果本身意味着没有发生任何故障 - 一切都按计划进行。

    另一方面,当方法抛出异常时,我需要解析一个 special 对象来找出问题所在。我不解析结果,因为没有结果

    一个例子:

    public void proceedWhenError() {
       final ServiceState state = getServiceState();
    
       if (state != ServiceState.RUNNING) {
          logger.debug("The service is not running, but it's alright.");
       }
       // do stuff
    }
    
    public void doNotProceedWhenError() {
       final ServiceState state = getServiceState();
    
       if (state != ServiceState.RUNNING) {
          throw new IllegalStateException("The service is not running...");
       }
       // do stuff
    }
    
    private ServiceState getServiceState() {
        try {
            service.connect();
            return ServiceState.RUNNING;
        catch(Exception e) {
            // determine the state by parsing the exception
            // and return it
            return getStateFromException(e);
        }
    }
    

    如果服务抛出的异常很重要和/或在其他地方处理,则可以将其与状态一起保存到 ServiceResponse 对象中:

    class ServiceResponse {
    
        private final ServiceState state;
        private final Exception exception;
    
        public ServiceResponse(ServiceState state, Exception exception) {
            this.state = state;
            this.exception = exception;
        }
    
        public static ServiceResponse of(ServiceState state) {
            return new ServiceResponse(state, null);
        }
    
        public static ServiceResponse of(Exception exception) {
            return new ServiceResponse(null, exception);
        }
    
        public ServiceState getState() {
            return state;
        }
    
        public Exception getException() {
            return exception;
        }
    
    }
    

    现在,使用ServiceResponse,这些方法可能如下所示:

    public void proceedWhenError() {
       final ServiceResponse response = getServiceResponse();
    
       final ServiceState state = response.getState();
       final Exception exception = response.getException();
    
       if (state != ServiceState.RUNNING) {
          logger.debug("The service is not running, but it's alright.", exception);
       }
       // do stuff
    }
    
    public void doNotProceedWhenError() {
       final ServiceResponse response = getServiceResponse();
    
       final ServiceState state = response.getState();
       final Exception exception = response.getException();
    
       if (state != ServiceState.RUNNING) {
          throw new IllegalStateException("The service is not running...", exception);
       }
       // do stuff
    }
    
    private ServiceResponse getServiceResponse() {
        try {
            service.connect();
            return ServiceResponse.of(ServiceState.RUNNING);
        catch(Exception e) {
            // or -> return ServiceResponse.of(e);
            return new ServiceResponse(getStateFromException(e), e);
        }
    }
    

    【讨论】:

      【解决方案5】:

      返回异常确实是一种反模式,因为异常应该为执行中的错误保留,而不是描述服务的条件。

      想象一下serviceUp() 的代码中是否存在导致它抛出NullPointerException 的错误。现在想象错误在服务中,并且从connect() 抛出相同的NullPointerException

      明白我的意思了吗?

      另一个原因是需求的变化。

      目前,服务有两种情况:向上或向下。
      目前。

      明天,你将有服务的三个条件:上,下。或带有警告的功能。第二天,您还希望该方法以 json 格式返回有关服务的详细信息.....

      【讨论】:

        【解决方案6】:

        明显的认知失调是这里的反模式。读者会看到您使用异常进行流控制,而开发人员会立即尝试对其进行重新编码,使其不会。

        我的直觉建议采用如下方法:

        // Use an action name instead of a question here because it IS an action.
        private void bringServiceUp() throws Exception {
        
        }
        
        // Encapsulate both a success and a failure into the result.
        class Result {
            final boolean success;
            final Exception failure;
        
            private Result(boolean success, Exception failure) {
                this.success = success;
                this.failure = failure;
            }
        
            Result(boolean success) {
                this(success, null);
            }
        
            Result(Exception exception) {
                this(false, exception);
            }
        
            public boolean wasSuccessful() {
                return success;
            }
        
            public Exception getFailure() {
                return failure;
            }
        }
        
        // No more cognitive dissonance.
        private Result tryToBringServiceUp() {
            try {
                bringServiceUp();
            } catch (Exception e) {
                return new Result(e);
            }
            return new Result(true);
        }
        
        // Now these two are fine.
        public void proceedWhenError() {
            Result result = tryToBringServiceUp();
            if (result.wasSuccessful()) {
                // do stuff
            } else {
                logger.debug("Exception happened, but it's alright.", result.getFailure());
                // do stuff
            }
        }
        
        public void doNotProceedWhenError() throws IllegalStateException {
            Result result = tryToBringServiceUp();
            if (result.wasSuccessful()) {
                // do stuff
            } else {
                // do stuff
                throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", result.getFailure());
            }
        }
        

        【讨论】:

          【解决方案7】:

          如果方法的调用者预期操作可能成功或失败,并准备处理任何一种情况,则该方法应通常通过返回值指示操作是否失败。如果调用者没有准备好有效地处理失败,那么发生失败的方法应该抛出异常,而不是要求调用者添加代码来这样做。

          一个问题是,有些方法的调用者准备好优雅地处理失败,而另一些则不然。我首选的方法是让这些方法接受一个可选的回调,在失败的情况下调用该回调;如果没有提供回调,默认行为应该是抛出异常。在调用者准备处理故障的情况下,这种方法将节省构造异常的成本,同时将未准备好处理的调用者的负担降至最低。这种设计的最大困难是决定这样一个回调应该采用什么参数,因为以后更改这些参数往往很困难。

          【讨论】:

            【解决方案8】:

            您不能说 2) 意见是错误的,因为工作流将按照您希望的方式运行。从逻辑的角度来看,它会按照需要运行,所以它是正确的。

            但这是一种非常奇怪且不推荐的方式。首先是因为 Exception 不是为了做到这一点而设计的(这意味着你正在做一个反模式)。这是一个特定的对象,旨在投掷和捕捉。所以很奇怪,宁愿按设计使用它,选择返回它,而是使用 if 来捕获它。此外,您将(也许可以忽略不计)面临性能问题,而不是使用简单的布尔值作为标志,而是实例化整个对象。

            最后也不推荐,因为如果函数的目标是获取某些东西(这不是你的情况),函数应该返回一些东西。你应该把它重新设计成一个启动服务的函数,然后它什么也不返回,因为它不会被调用来获取东西,如果发生错误它会抛出异常。如果您想知道您的服务是否工作,请创建一个旨在为您提供此信息的函数,例如 public boolean isServiceStarted()

            【讨论】:

              【解决方案9】:

              有三个(惯用的)来处理运行时可能失败的函数。

              1。返回布尔值

              第一个是返回boolean。这种风格广泛用于 C 和 C 风格的 API,如 PHP。这会导致 - 例如在 PHP 中 - 经常编写这样的代码:

              if (xyz_parse($data) === FALSE)
                  $error = xyz_last_error();
              

              这样做的缺点是显而易见的,这就是它越来越过时的原因,尤其是在 OOP 语言和具有异常处理的语言中。

              2。使用中介/状态/结果对象

              如果你不使用异常,你仍然有机会在 OOP 语言中返回描述状态对象

              例如,如果您收到来自用户的输入并想要验证输入,那么您事先知道您可能会得到垃圾,并且结果经常不会验证,你可能会写这样的代码:

              ValidationResult result = parser.validate(data);
              if (result.isValid())
                  // business logic
              else
                  error = result.getvalidationError();
              

              3。使用例外

              Java 执行此操作,正如您在套接字中所展示的那样。原因是创建连接应该成功并且仅在需要特殊处理的异常情况下失败。

              应用它

              在您的情况下,我会直接抛出异常,甚至删除辅助方法。您的辅助方法命名错误。它实际上并不查询服务是否启动(顾名思义),而只是连接

              使用(检查)异常

              让我们假设连接是您的方法实际上所做的。在这种情况下,我们将其命名为connectToService 更为贴切,并使其直接抛出异常:

              public void connectToService() thows IOException {
                  // yada yada some init stuff, whatever
                  socket.connect();
              }
              
              public void proceedWhenError() {
                 try {
                      connectToService();
                 } else {
                    logger.debug("Exception happened, but it's alright.", exception)
                    // do stuff
                 }
              }
              
              public void doNotProceedWhenError() throws IllegalStateException {
                  try  {
                      connectToService();
                      // do stuff
                  }
                  catch(IOException e) {
                    throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
                 }
              }
              

              使用boolean

              另一方面,您的serviceUp仍然命名错误,最好称为isServiceUp)实际上查询是否服务是否正在运行(可能已由其他人启动)。在这种情况下,使用布尔值是正确的方法

              public boolean isServiceUp {
                  return ...; // query the service however you need
              }
              

              当然,这指的是使用布尔值的第一个解决方案,当您还需要知道为什么服务未启动时,它并没有真正的帮助。

              使用中介/结果/状态对象

              因此,让我们摒弃这种有限的方法并使用中介/结果/状态对象对其进行改造:

              class ServiceState {
                  enum State { CONNECTED, WAITING, CLOSED; }
              
                  State state;
                  IOException exception;
                  String additionalMessage;
              
                  public ServiceState (State state, IOException ex, String msg) {
                       [...] // ommitted for brevity
                  }
              
                 // getters ommitted for brevity
              }
              

              辅助函数将变为getServiceState

              public ServiceState getServiceState() {
                  // query the service state however you need & return
              }
              
              public void proceedWhenError() {
                 ServiceState state = getServiceState();
              
                 if (state.getState() == State.CONNECTED) {
                    // do stuff
                 } else {
                    logger.debug("Exception happened, but it's alright.", state.getException())
                    // do stuff
                 }
               }
              
              public void doNotProceedWhenError() {
                  ServiceState state = getServiceState();
              
                 if (state.getState() == State.CONNECTED) {
                    // do stuff
                 } else {
                    // do stuff
                    throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
                 }
              }
              

              请注意,您也在proceedWhenError 中重复自己,整个代码可以简化为:

              public void proceedWhenError() {
                 ServiceState state = getServiceState();
              
                if (state.getState() != State.CONNECTED) {
                    logger.debug("Exception happened, but it's alright.", state.getException())
              
                 }
               // do stuff
              }
              

              何时使用第二种情况以及何时使用第三种情况存在一些争议。有些人认为异常应该是异常的,并且您不应该在设计时考虑到异常的可能性,并且几乎总是使用第二种选择。没事儿。但是我们已经检查了 Java 中的异常,所以我认为没有理由不使用它们。我使用检查的异常 当基本假设是调用应该成功(如使用套接字),但有可能失败时,当非常不清楚调用是否应该成功(如验证数据)时,我使用第二个选项。但对此众说纷纭。

              这也取决于你的辅助方法实际上做了什么。 isServiceUp 暗示它查询某些状态,但不会改变它。在这种情况下,返回一个状态对象显然是处理这种情况的惯用方式。

              但是您的实现显示辅助方法连接到服务。在这种情况下,至少在 java 中,处理它的惯用方法是抛出(检查的)异常 - 但您仍然可以使用结果对象来证明是合理的。

              不过,不建议只返回boolean,因为它太有限了。如果所有您需要知道是否服务是否正在运行(并且对原因不感兴趣),这样的方法可能仍然不过很有用(并且可以在幕后实现为仅执行 return getServiceState() == State.SUCCESS 的辅助方法)。


              使用异常来控制工作流是一种反模式

              那么,让我们看看,什么是异常?

              定义:异常是在程序执行期间发生的一个事件,它扰乱了程序指令的正常流程

              来源:https://docs.oracle.com/javase/tutorial/essential/exceptions/definition.html

              异常的定义是它是一个扰乱(从而改变)程序工作流程的事件。如果按照定义使用它是一种反模式,那么整个语言特性本身就必须被认为是一种反模式。有些人确实相信异常是不好的,并且一些一些有效的论据,但一般来说,异常被认为是有用的构造并被视为处理异常工作流程的有用工具。

              抛出异常当然不是反模式。 返回它,另一方面,是。因此,是的,您的代码 - 如所示 - 是非惯用的。

              【讨论】:

                【解决方案10】:

                这是一个反模式:

                catch(Exception e) {
                   return e;
                }
                

                返回Throwable 的唯一合理借口是向预期失败的快速路径提供详细的失败信息,而无需支付异常处理的代价1。作为奖励,如果调用者不知道如何处理这种特殊情况,它可以很容易地转换为抛出的异常。

                但在您的情况下,该成本已经支付。代表调用者捕获异常对任何人都没有任何好处。


                1学究式地,捕获异常(查找匹配的处理程序、堆栈展开)的直接成本非常低或无论如何都需要完成。与返回值相比,try/catch 的大部分成本是附带操作,例如构建堆栈跟踪。因此,未抛出的异常对象能否为有效的错误数据提供良好的存储取决于这种附带工作是在构造时完成还是在抛出时完成。因此返回异常对象是否合理可能在不同的托管平台之间有所不同。

                【讨论】:

                  【解决方案11】:

                  前面cmets中提到“异常是一个事件”,我们得到的异常对象只是事件的一个细节。 一旦 Exception 在“catch”块中被捕获并且没有重新抛出,事件就结束了。 发布您只有一个细节对象而不是异常,尽管该对象属于异常/可抛出类。

                  返回异常对象可能是有用的,因为它包含细节,但它会增加歧义/混乱,因为调用方法不是“处理异常和基于异常控制流”。它只是使用返回对象的细节来做出决定。

                  因此,在我看来,一种更合乎逻辑且不易混淆的方法是根据异常返回布尔值/枚举,而不是简单地返回异常对象。

                  【讨论】:

                    猜你喜欢
                    • 1970-01-01
                    • 2017-05-17
                    • 1970-01-01
                    • 1970-01-01
                    • 1970-01-01
                    • 1970-01-01
                    • 1970-01-01
                    • 1970-01-01
                    • 1970-01-01
                    相关资源
                    最近更新 更多