catch语句中的附加try语句 - 代码味道?

问题描述 投票:5回答:15

情况:

我的应用程序需要处理业务规则的第一步(初始的try-catch语句)。如果进程在该步骤中调用辅助方法时发生某个错误,我需要切换到catch语句中的第二个进程。备份过程使用相同的帮助程序方法。如果在第二个进程中发生同样的错误,我需要停止整个进程并抛出异常。

执行:

我打算在第一张try-catch声明的catch声明中插入另一个try-catch声明。

//run initial process
try
{
    //initial information used in helper method
    string s1 = "value 1";

    //call helper method
    HelperMethod(s1);
}
catch(Exception e1)
{
    //backup information if first process generates an exception in the helper method
    string s2 = "value 2";

    //try catch statement for second process.
    try
    {
        HelperMethod(s2);
    }
    catch(Exception e2)
    {
        throw e2;
    }
}

在这个实现中避免代码味道的正确设计模式是什么?

我引起了一些混淆,并忽略了当第一个进程失败并切换到第二个进程时,它会向辅助方法发送不同的信息。我已更新方案以反映整个过程。

c# design-patterns try-catch
15个回答
12
投票

如果HelperMethod需要第二次尝试,那么这没有什么直接的错误,但是catch中的代码试图做得太多,它会破坏e2中的堆栈跟踪。

你只需要:

try
{
    //call helper method
    HelperMethod();
}    
catch(Exception e1)
{
    // maybe log e1, it is getting lost here
    HelperMethod();
}

0
投票

这是另一种模式:

// set up state for first attempt
if(!HelperMethod(false)) {
        // set up state for second attempt
        HelperMethod(true);
        // no need to try catch since you're just throwing anyway
}

在这里,HelperMethod

bool HelperMethod(bool throwOnFailure)

并且返回值表示是否成功发生(即,false表示失败,true表示成功)。你也可以这样做:

// could wrap in try/catch
HelperMethod(2, stateChanger);

HelperMethod在哪里

void HelperMethod(int numberOfTries, StateChanger[] stateChanger)

其中numberOfTries表示在抛出异常之前尝试的次数,StateChanger[]是一个代理数组,它将在调用之间改变状态(即,在第一次尝试之前调用stateChanger[0],在第二次尝试之前调用stateChanger[1],等等。 )

最后一个选项表明您可能有一个有臭味的设置。看起来封装此过程的类负责跟踪状态(查找哪个员工)以及查找员工(HelperMethod)。通过SRP,这些应该是分开的。

当然,你需要一个catch比你现在更具体的例外(不要抓住基类Exception!)如果你需要在记录,清理等之后重新抛出异常,你应该只使用throw而不是throw e


0
投票

您可以模拟C#的TryParse方法签名:

class Program
{
    static void Main(string[] args)
    {
        Exception ex;
        Console.WriteLine("trying 'ex'");
        if (TryHelper("ex", out ex))
        {
            Console.WriteLine("'ex' worked");
        }
        else
        {
            Console.WriteLine("'ex' failed: " + ex.Message);
            Console.WriteLine("trying 'test'");
            if (TryHelper("test", out ex))
            {
                Console.WriteLine("'test' worked");
            }
            else
            {
                Console.WriteLine("'test' failed: " + ex.Message);
                throw ex;
            }
        }
    }

    private static bool TryHelper(string s, out Exception result)
    {
        try
        {
            HelperMethod(s);
            result = null;
            return true;
        }
        catch (Exception ex)
        {
            // log here to preserve stack trace
            result = ex;
            return false;
        }
    }

    private static void HelperMethod(string s)
    {
        if (s.Equals("ex"))
        {
            throw new Exception("s can be anything except 'ex'");
        }
    }

}

0
投票

另一种方法是展平try / catch块,如果您正在使用一些异常快乐的API,则非常有用:

public void Foo()
{
  try
  {
    HelperMethod("value 1");
    return; // finished
  }
  catch (Exception e)
  {
     // possibly log exception
  }

  try
  {
    HelperMethod("value 2");
    return; // finished
  }
  catch (Exception e)
  {
     // possibly log exception
  }

  // ... more here if needed
}

0
投票

重试的选项(大多数人可能会火焰)将使用goto。 C#没有过滤异常,但可以以类似的方式使用。

const int MAX_RETRY = 3;

public static void DoWork()
{
    //Do Something
}

public static void DoWorkWithRetry()
{
    var @try = 0;
retry:
    try
    {
        DoWork();
    }
    catch (Exception)
    {
        @try++;
        if (@try < MAX_RETRY)
            goto retry;
        throw;
    }
}

0
投票

在这种情况下,你知道这个“异常”可能会发生,所以我更喜欢一个简单的方法,为未知事件留下例外。

//run initial process
try
{
    //initial information used in helper method
    string s1 = "value 1";

    //call helper method
    if(!HelperMethod(s1))
    {
        //backup information if first process generates an exception in the helper method
        string s2 = "value 2";
        if(!HelperMethod(s2))
        {
          return ErrorOfSomeKind;
        }
    }
    return Ok;
}
catch(ApplicationException ex)
{
    throw;
}

0
投票

我知道我最近完成了上面嵌套的try catch处理解码数据,其中两个第三方库在解码失败时抛出异常(尝试json解码,然后尝试base64解码),但我的首选是让函数返回一个值可以检查。

我通常只使用抛出异常来提前退出,并通知链上有关错误的内容,如果它对进程是致命的。

如果函数无法提供有意义的响应,那通常不是致命的问题(与错误的输入数据不同)。

看起来嵌套try catch的主要风险是你最终还会捕获可能发生的所有其他(可能很重要的)异常。


4
投票

我不会说它很糟糕,虽然我几乎肯定会将第二块代码重构为第二种方法,所以要保持它的可理解性。并且可能会抓住比Exception更具体的东西。第二次尝试有时是必要的,特别是像Dispose()实现本身可能抛出的东西(WCF,我在看着你)。


2
投票

将try-catch放在父级try-catch的catch中的一般想法对我来说似乎不是代码味道。我可以想到这样做的其他合理原因 - 例如,在清理失败的操作时,您不希望再抛出另一个错误(例如清理操作也失败)。但是,你的实现为我提出了两个问题:1)Wim的评论,2)你真的想完全忽视操作最初失败的原因(e1例外)吗?无论第二个进程成功还是失败,您的代码都不会对原始异常执行任何操作。


1
投票

一般来说,这不是问题,也不是我所知道的代码味道。

话虽如此,您可能希望在第一个辅助方法中处理错误,而不是仅仅抛出它(因此,在那里处理对第二个辅助方法的调用)。只有这样才有意义,但这是一种可能的改变。


1
投票

是的,更通用的模式是基本方法包括接受int attempt参数的重载,然后有条件地递归调用自身。

   private void MyMethod (parameterList)
   {  MyMethod(ParameterList, 0)l }

   private void MyMethod(ParameterList, int attempt)
   {
      try { HelperMethod(); }
      catch(SomeSpecificException)
      {
          if (attempt < MAXATTEMPTS)
              MyMethod(ParameterList, ++attempt);
          else throw;
      }
   }

0
投票

它不应该那么糟糕。只是清楚地记录你为什么要这样做,并且最明确地尝试捕获更具体的异常类型。


0
投票

如果你需要一些重试机制,你可能想要探索不同的技术,循环延迟等。


0
投票

如果你在catch中调用一个不同的函数会更清楚一点,这样读者就不会认为你只是重新尝试相同的函数了。如果您的示例中没有显示正在发生的状态,则应至少仔细记录。

你也不应该像那样throw e2;:你应该只是throw;,如果你要处理你所捕获的例外。如果没有,你不应该尝试/捕获。

如果你没有引用e1,你应该只是catch (Exception)或更好的catch (YourSpecificException)


0
投票

如果您正在尝试从某种瞬态错误中恢复,那么您需要注意如何实现它。

例如,在您使用SQL Server镜像的环境中,您连接的服务器可能会停止作为主中间连接。

在这种情况下,您的应用程序尝试重新连接并重新执行新主服务器上的任何语句可能是有效的 - 而不是立即将错误发送回调用方。

您需要小心确保您调用的方法没有自己的自动重试机制,并且您的调用者知道您的方法中内置了自动重试。如果无法确保这种情况,可能会导致导致大量重试尝试的情况,从而导致共享资源(例如数据库服务器)过载。

您还应该确保捕获特定于您尝试重试的瞬态错误的异常。因此,在我给出的示例中,SqlException,然后检查错误是否SQL连接失败,因为主机不再是主服务器。

如果您需要多次重试,请考虑进行“自动退避”重试延迟 - 第一次失败立即重试,第二次失败后延迟(比方说)1秒,然后加倍到最多(比方说)90秒。这应该有助于防止资源过载。

我还建议重构你的方法,这样你就没有内部的try / catch。

例如:

bool helper_success = false; 
bool automatic_retry = false; 
//run initial process
try
{
    //call helper method
    HelperMethod();
    helper_success = true; 
}
catch(Exception e)
{
    // check if e is a transient exception. If so, set automatic_retry = true 
} 


if (automatic_retry)
{    //try catch statement for second process.
    try
    {
        HelperMethod();
    }
    catch(Exception e)
    {
        throw;
    }
}
© www.soinside.com 2019 - 2024. All rights reserved.