【问题标题】:C++ code purityC++ 代码纯度
【发布时间】:2011-05-09 12:33:07
【问题描述】:

我在 C++ 环境中工作并且:
a) 我们被禁止使用异常
b) 它是评估大量不同类型请求的应用程序/数据服务器代码

我有一个简单的类封装了服务器操作的结果,它也被内部用于那里的许多功能。

class OpResult
{
  .....
  bool succeeded();
  bool failed(); ....
  ... data error/result message ...
};

当我试图让所有功能变得小而简单时,就会出现很多这样的块:

....
OpResult result = some_(mostly check)function(....);
if (result.failed())
  return result;
...

问题是,让宏看起来像这样并在任何地方使用它是不好的做法吗?

#define RETURN_IF_FAILED(call) \
  {                            \
    OpResult result = call;    \
    if (result.failed())       \
      return result;           \
  }

我知道有人会说它讨厌,但有更好的方法吗? 您还有什么其他方法可以处理结果并避免大量臃肿的代码?

【问题讨论】:

  • 如果需要清理怎么办?
  • 好吧,我接受的答案被不喜欢的随机版主单方面删除了。

标签: c++ error-handling refactoring


【解决方案1】:

这是一个权衡。您正在交易代码大小以混淆逻辑。我更喜欢将逻辑保留为可见。

我不喜欢这种类型的宏,因为它们会破坏 Intellisense(在 Windows 上)和程序逻辑的调试。尝试在函数中的所有 10 个return 语句上设置断点——不是检查,只是return。尝试单步执行宏中的代码。

最糟糕的是,一旦你接受了这一点,就很难反驳一些程序员喜欢在常见的小任务中使用的 30 行怪物宏,因为它们“澄清了事情”。我已经看到了通过四个级联宏以这种方式处理不同异常类型的代码,导致源文件中有 4 行,而宏实际上扩展到超过 100 行。现在,您是否正在减少代码膨胀?不。用宏无法轻易分辨。

另一个反对宏的一般论点,即使在这里并不明显适用,是能够将它们嵌套在难以破译的结果中,或者传入导致奇怪但可编译的参数的参数,例如在使用参数两次的宏中使用++x。我总是知道我对代码的立场,而对于宏我不能这么说。

编辑:我应该补充的一条评论是,如果您确实一遍又一遍地重复此错误检查逻辑,那么代码中可能存在重构机会。如果确实适用,则不是保证,而是减少代码膨胀的更好方法。寻找重复的调用序列并将公共序列封装在它们自己的函数中,而不是单独处理每个调用的处理方式。

【讨论】:

  • 我同意宏应该始终是最后使用的东西,当尝试了所有其他“澄清事情”的方法时,它应该很简单,但是复制粘贴宏真的更好吗?而不是它的用法?
  • 它是否增加代码膨胀取决于是否有这些宏的替代品。如果宏的替代方法是手动编写 100 行,那么宏不会增加代码膨胀。如果有替代方案,那么是的,宏是有问题的。
  • @Peter - 恭敬地不同意。我永远不会将那么多行代码放在生产代码的宏中。测试代码是另一回事。调试这样的代码实际上是不可能的,而且当代码的真实性质被隐藏到这样的程度时,无论采用何种混淆机制,这都是一种代码气味。
【解决方案2】:

实际上,我更喜欢其他解决方案。问题是内部调用的结果不一定是外部调用的有效结果。例如,内部故障可能是“找不到文件”,但外部故障可能是“配置不可用”。因此我的建议是重新创建OpResult(可能将“内部”OpResult 打包到其中以便更好地调试)。这一切都朝着 .NET 中“InnerException”的方向发展。

从技术上讲,就我而言,宏看起来像

#define RETURN_IF_FAILED(call, outerresult) \
  {                                         \
    OpResult innerresult = call;            \
    if (innerresult.failed())               \
    {                                       \
        outerresult.setInner(innerresult);  \
        return outerresult;                 \
    }                                       \
  }

这个解决方案需要一些内存管理等。

一些纯粹主义者认为,没有显式返回会妨碍代码的可读性。但在我看来,将明确的 RETURN 作为宏名称的一部分足以防止任何熟练且细心的开发人员感到困惑。


我的观点是,这样的宏不会混淆程序逻辑,反而会使程序更清晰。使用这样的宏,您可以以清晰简洁的方式声明您的意图,而另一种方式似乎过于冗长,因此容易出错。让维护者在脑海中解析相同的构造 OpResult r = call(); if (r.failed) return r 是在浪费他们的时间。

另一种不提前返回的方法是将CHECKEDCALL(r, call)#define CHECKEDCALL(r, call) do { if (r.succeeded) r = call; } while(false) 之类的模式应用于每个代码行。这在我看来要糟糕得多,而且绝对容易出错,因为人们在添加更多代码时往往会忘记添加 CHECKEDCALL()

有一个受欢迎的需要用宏检查返回(或所有东西)似乎是我缺少语言功能的轻微迹象。

【讨论】:

  • 类似的东西也可能有用。
  • 如果你需要这个,然后将这些内部调用放入它们自己的(内联)函数中,外部函数解释它们的结果。那么就不需要这个宏黑客了。
  • @sbi:我在答案中的代码只是一个提示。实际的生产代码使用了一些适当的辅助函数。
【解决方案3】:

只要宏定义位于实现文件中并且在不必要时未定义,我就不会害怕

// something.cpp

#define RETURN_IF_FAILED() /* ... */

void f1 () { /* ... */ }
void f2 () { /* ... */ }

#undef RETURN_IF_FAILED

但是,我只会在排除所有非宏观解决方案后才使用它。

【讨论】:

    【解决方案4】:

    我同意Steve's POV

    我首先想到,至少把宏缩小到

    #define RETURN_IF_FAILED(result) if(result.failed()) return result;
    

    但后来我突然想到这已经单行了,所以宏确实没有什么好处。


    我认为,基本上,您必须在可写性和可读性之间进行权衡。宏绝对更容易编写。然而,它是否也更容易阅读是一个悬而未决的问题。后者是一个相当主观的判断。尽管如此,客观地使用宏确实混淆了代码。


    最终,根本问题是您不能使用异常。你还没有说这个决定的原因是什么,但我当然希望他们值得这个造成的问题。

    【讨论】:

    • 这样,你必须调用函数,然后调用宏,而且你必须在任何地方都使用块,或者小心不要重复结果变量。
    • 您示例中的宏不是最佳的,因为它会在代码中带来错误,例如if (cond) RETURN_IF_FAILED(...) else do_something_else;。您可能想在周围添加通常的do { ... } while(0)
    • @Vlad: 我在争论 反对 那个宏,这就是为什么我没有考虑它。
    • @Marwin:您不必小心,因为如果您失败,编译器会捕获它。
    【解决方案5】:

    可以使用 C++0x lambdas 完成。

    template<typename F> inline OpResult if_failed(OpResult a, F f) {
        if (a.failed())
            return a;
        else
            return f();
    };
    
    OpResult something() {
        int mah_var = 0;
        OpResult x = do_something();
        return if_failed(x, [&]() -> OpResult {
            std::cout << mah_var;
            return f;
        });
    };
    

    如果你聪明又不顾一切,你可以用同样的技巧来处理普通的物体。

    【讨论】:

      【解决方案6】:

      在我看来,在宏中隐藏 return 语句是个坏主意。 “代码混淆”(我喜欢这个词……!)达到了最高水平。对于此类问题,我通常的解决方案是将函数执行聚合在一个地方并以以下方式控制结果(假设您有 5 个空函数):

      std::array<std::function<OpResult ()>, 5>  tFunctions = {
       f1, f2, f3, f4, f5
      };
      
      auto tFirstFailed = std::find_if(tFunctions.begin(), tFunctions.end(), 
          [] (std::function<OpResult ()>& pFunc) -> bool {
              return pFunc().failed();
          });
      
      if (tFirstFailed != tFunctions.end()) {
       // tFirstFailed is the first function which failed...
      }
      

      【讨论】:

      • 当宏名以RETURN_IF_...开头时,很难将这个hiding称为return语句
      【解决方案7】:

      如果调用失败,结果中是否有任何实际有用的信息?

      如果没有,那么

      static const error_result = something;
      
      if ( call().failed() ) return error_result; 
      

      足够了。

      【讨论】:

        【解决方案8】:

        10 年后,如果我有一台时光机,我会满意地回答我自己的问题……

        我在新项目中多次遇到类似情况。即使允许例外,我也不想总是将它们用于“正常失败”。

        我最终找到了一种编写此类语句的方法。

        对于包含消息的通用结果,我使用这个:

        class Result
        {
        public:
          enum class Enum
          {
            Undefined,
            Meaningless,
            Success,
            Fail,
          };
          static constexpr Enum Undefined = Enum::Undefined;
          static constexpr Enum Meaningless = Enum::Meaningless;
          static constexpr Enum Success = Enum::Success;
          static constexpr Enum Fail = Enum::Fail;
        
          Result() = default;
          Result(Enum result) : result(result) {}
          Result(const LocalisedString& message) : result(Fail), message(message) {}
          Result(Enum result, const LocalisedString& message) : result(result), message(message) {}
          bool isDefined() const { return this->result != Undefined; }
          bool succeeded() const { assert(this->result != Undefined); return this->result == Success; }
          bool isMeaningless() const { assert(this->result != Undefined); return this->result == Enum::Meaningless; }
          bool failed() const { assert(this->result != Undefined); return this->result == Fail; }
          const LocalisedString& getMessage() const { return this->message; }
        
        private:
          Enum result = Undefined;
          LocalisedString message;
        };
        

        然后,我有一个这种形式的特殊助手类,(其他返回类型类似)

        class Failed
        {
        public:
          Failed(Result&& result) : result(std::move(result)) {}
          explicit operator bool() const { return this->result.failed(); }
          operator Result() { return this->result; }
          const LocalisedString& getMessage() const { return this->result.getMessage(); }
        
          Result result;
        };
        

        当这些组合在一起时,我可以编写如下代码:

        if (Failed result = trySomething())
          showError(result.getMessage().str());
        

        是不是很漂亮?

        【讨论】:

          猜你喜欢
          • 1970-01-01
          • 2012-09-06
          • 2012-12-01
          • 2012-05-09
          • 2020-05-14
          • 2016-05-03
          • 2011-04-06
          • 2014-05-13
          • 2012-05-04
          相关资源
          最近更新 更多