2

我想重写一个嵌套if语句太多的方法。

我想出了这种方法，并希望得到您的意见：

public void MyMethod()
{
   bool hasFailed = false;

   try
   {
      GetNewOrders(out hasFailed);

      if(!hasFailed)
          CheckInventory(out hasFailed);

      if(!hasFailed)
          PreOrder(out hasFailed);              

      // etc
   }
   catch(Exception ex)
   {
   }
   finally
   {
      if(hasFailed)
      {
           // do something
      }
   }
}
4

10 回答 10

9

我做了类似的事情，但没有异常处理：

BOOL ok = CallSomeFunction();
if( ok ) ok = CallSomeOtherFunction();
if( ok ) ok = CallYetAnotherFunction();
if( ok ) ok = WowThatsALotOfFunctions();
if( !ok ) {
    // handle failure
}

或者，如果你想变得聪明：

BOOL ok = CallSomeFunction();
ok &= CallSomeOtherFunction();
ok &= CallYetAnotherFunction();
...

如果您仍然使用异常，为什么需要该hasFailed变量？

于 2009-01-09T14:20:50.177 回答
7

并不真地。如果您的“catch”块捕获到错误，您的方法应该引发异常。

于 2009-01-09T14:18:30.510 回答
5

据我所知，这是一个级联步骤的示例，如果第一个和第一个和第二个有效，则将执行第二个和第三个步骤，即返回 hasFailed==false。

使用模板方法和装饰器设计模式可以使这段代码更加优雅。

你需要一个接口、具体实现、抽象类和抽象类的几个子类。

public interface Validator {
    public boolean isValid();
}

public class GetNewOrders implements Validator {
    public boolean isValid() {
       // same code as your GetNewOrders method
    }
}

public abstract class AbstractValidator implements Validator {
    private final Validator validator;

    public AbstractValidator(Validator validator) {
        this.validator = validator;
    }
    protected boolean predicate();
    protected boolean isInvalid();

    public final boolean isValid() {
        if (!this.validator.isValid() && predicate() && isInvalid())
            return false;
        return true;
    }
}

public class CheckInventory extends AbstractValidator {
    public CheckInventory(Validator validator) {
        super(validator);
    }
    @Override
    public boolean predicate() {
        return true;
    }
    @Override
    public boolean isInvalid() {
        // same code as your CheckInventory method
    }
}

public class PreOrder extends AbstractValidator {
    public CheckInventory(Validator validator) {
        super(validator);
    }
    @Override
    public boolean predicate() {
        return true;
    }
    @Override
    public boolean isInvalid() {
        // same code as your PreOrder method
    }
}

现在您的方法看起来更加优雅：

public void MyMethod() {
    bool success = false;
    try {
        Validator validator = new GetNewOrders();
        validator = new CheckInventory(validator);
        validator = new PreOrder(validator);
        success = validator.isValid();
    } finally {
        if (!success) {
            // do something
        }
    }
}

Validator 对象可以在一行中创建，但我更喜欢这种样式，因为它使验证的顺序很明显。在链中创建新的验证链接是继承 AbstractValidator 类和实现谓词和 isInvalid 方法的问题。

于 2009-01-09T14:46:46.663 回答
3

没有评论 try/catch 的东西，因为我真的不知道那里发生了什么，我会改变它，所以被调用的方法返回 true/false 表示成功，然后根据布尔短路检查它们以避免调用如果前面的方法失败，后面的方法。

public void MyMethod()
{
    bool success = false;
    try
    {
         success = GetNewOrders()
                   && CheckInventory()
                   && PreOrder();
         // etc
    }
    catch(Exception ex)   {   }
    finally
    {
        if(!success)
        {
        }
    }
}
于 2009-01-09T14:29:36.883 回答
1

这对我来说真的不好看。hasFailed 变量的使用确实不好。如果 GetNewOrders 因异常而失败，例如，您最终会进入带有 hasFailed = false 的 catch 块！

与此处的其他答案相反，我相信布尔“hasFailed”可能有合法用途，这并不例外。但我真的不认为你应该将这样的条件混合到你的异常处理程序中。

于 2009-01-09T14:18:18.530 回答
0

我知道我可能会复制一些帖子：有什么问题else？您也可以使用惰性求值 ( a() && b()) 来链接方法 - 但这依赖于将状态作为返回值给出，无论如何恕我直言，这更具可读性。

我不同意你应该引发异常的海报，因为如果发生程序错误或程序由于操作而进入异常状态，则应该引发异常。异常不是业务逻辑。

于 2009-01-09T14:34:31.913 回答
0

我会这样做：

public void MyMethod()
{
bool success = false;   
try
   {

      GetNewOrders(); // throw GetNewOrdersFailedException    
      CheckInventory(); // throw CheckInventoryFailedException
      PreOrder(); // throw PreOrderFailedException  
      success = true;            
   }
   catch( GetNewOrdersFailedException e)
   {
     // Fix it or rollback
   }
   catch( CheckInventoryFailedException e)
   {
     // Fix it or rollback
   }
   catch( PreOrderFailedException e)
   {
     // Fix it or rollback
   }
   finally
   {
      //release resources;
   }
}

扩展异常相当简单，

公共 NewExecption : BaseExceptionType {}

于 2009-01-09T14:35:14.563 回答
0

好吧，我不喜欢看起来获取订单列表然后处理它们，然后在发生错误时停止处理它们的代码，什么时候它肯定应该跳过该订单并移至下一个订单？唯一完全失败的是数据库（订单来源，预购目的地）死亡。我认为整个逻辑确实有点时髦，但也许那是因为我对您使用的语言没有经验。

try {
  // Get all of the orders here
  // Either in bulk, or just a list of the new order ids that you'll call the DB
  // each time for, i'll use the former for clarity.
  List<Order> orders = getNewOrders();

  // If no new orders, we will cry a little and look for a new job
  if (orders != null && orders.size() > 0) {
    for (Order o : orders) {
      try {
        for (OrderItem i : o.getOrderItems()) {
          if (checkInventory(i)) {
            // Reserve that item for this order
            preOrder(o, i);
          } else {
            // Out of stock, call the magic out of stock function
            failOrderItem(o, i);
          }
        }
      } catch (OrderProcessingException ope) {
        // log error and flag this order as requiring attention and
        // other things relating to order processing errors that aren't database related
      }
    }
  } else {
    shedTears();
  }
} catch (SQLException e) {
  // Database Error, log and flag to developers for investigation
}
于 2009-01-09T15:04:46.253 回答
0

您的新方法对于一组简单的指令来说并没有那么糟糕，但是当添加额外的步骤时会发生什么？您/您是否需要交易行为？（如果 PreOrder 失败了怎么办？或者如果 PreOrder 之后的下一步失败了？）

展望未来，我会使用命令模式： http ://en.wikipedia.org/wiki/Command_pattern

...并将每个动作封装为实现 Execute() 和 Undo() 的具体命令。

然后只需创建一个命令队列并循环直到失败或空队列。如果任何步骤失败，则只需停止并按先前命令的顺序执行 Undo()。简单的。

于 2009-01-09T15:20:50.760 回答
0

克里斯解决方案是最正确的。但我认为你不应该做超过你需要的事情。解决方案应该是可扩展的，这就足够了。

  1. 更改参数的值是一种不好的做法。
  2. 永远不要使用空的通用 catch 语句，至少添加一个注释为什么你这样做。
  3. 使方法抛出异常并在适当的地方处理它们。

所以现在它更加优雅:)

public void MyMethod()
{
   try
   {
      GetNewOrders();
      CheckInventory();
      PreOrder();
      // etc
   }
   finally
   {
      // do something
   }
}
于 2009-01-09T15:38:26.357 回答