c - 在不使用宏的情况下简化并减少代码量

Question

有时，在某些情况下，简单代码块的重复是不可避免的。为了说明，使用此示例代码：

注意：此代码仅用于说明目的，实际代码更大更复杂。它也可能包含错误，但这个问题的重点不是那个。

switch(cmd) {
    case CMD_BLOCK_READ:
        if(current_user != key) {
            ERROR("Access violation - invalid key!");
            res = CR_ACCESS_DENIED;
            break; 
        }
        if(current_state < STATE_BUSY) {
            WARN("Command %s is not allowed in this state!", cmd_name[cmd]);
            res = CR_NOT_PERMITTED;
            break;
        }
        if(ioctl(fd, HPI_CTL_BR) != 0) {
            WARN("Handshake failed (%s). Aborted!", strerror(errno));
            res = CR_TIME_OUT;
            goto post_resp;
        }
        if(block_read(id) != 0) {
            ERROR("Failed to read %d block (%s)! Aborted!", id, strerror(errno));
            res = CR_FAIL;
            goto send_nop;
        }

        res = CR_SUCCESS;
        break;
    case CMD_BLOCK_WRITE:
        if(current_user != key) {
            ERROR("Access violation - invalid key!");
            res = CR_ACCESS_DENIED;
            break; 
        }
        if(current_state < STATE_BUSY) {
            WARN("Command %s is not allowed in this state!", cmd_name[cmd]);
            res = CR_NOT_PERMITTED;
            break;
        }
        if(ioctl(fd, HPI_CTL_BR) != 0) {
            WARN("Handshake failed (%s). Aborted!", strerror(errno));
            res = CR_TIME_OUT;
            goto post_resp;
        }
        if(block_write(id) != 0) {
            ERROR("Failed to write %d block - %s. Command aborted!", id, strerror(errno));
            res = CR_FAIL;
            goto send_nop;
        }
        res = CR_SUCCESS;
        break;
    case CMD_REQ_START:
        if(current_state < STATE_READY) {
            WARN("Command %s is not allowed in this state!", cmd_name[cmd]);
            res = CR_NOT_PERMITTED;
            break;
        }
        state = STATE_BUSY;
        if(ioctl(fd, HPI_CTL_BR) != 0) {
            WARN("Handshake failed (%s). Aborted!", strerror(errno));
            res = CR_TIME_OUT;
            goto send_nop;
        }
        if(block_read(id) != 0) {
            ERROR("Failed to read %d block (%s)! Aborted!", id, strerror(errno));
            res = CR_FAIL;
            goto post_resp;
        }
        res = CR_SUCCESS;
        break;
    }

    /* The remaining 28 or so similar commands */
}

如您所见，由于细微差别和广泛使用break/goto语句，无法使用函数或内联。我通常做的是定义一些宏：

/* NOTE: DO NOT USE these macros outside of Big Switch */
#define CHECK_KEY(_key) \
   if(current_user != (_key)) \
   { \
      ERROR("Access violation!"); \
      res = CR_ACCESS_DENIED; \
      break; \
   }
#define CHECK_STATE(_state) \
   if(current_state < _state) \
   { \
      WARN("Command %s is not allowed in this state!", cmd_name[cmd]); \
      res = CR_NOT_PERMITTED; \
      break; \
   }

#define HANDSHAKE(_fail) \
   if(ioctl(fd, CTL_BR) != 0) \
   { \
      WARN("Handshake failed (%s). Aborted!", strerror(errno)); \
      res = CR_TIME_OUT; \
      goto _fail; \
   }

#define BLOCK_READ(_id, _fail) \
   if(block_read((int)(_id))!= 0) \
   { \
      ERROR("Failed to read %d block (%s)! Aborted!", (int)_id, strerror(errno)); \
      res = CR_FAIL; \
      goto _fail; \
   }

#define BLOCK_WRITE(_id, _fail) \
   if(block_write((int)(_id)) != 0) \
   { \
      ERROR("Failed to write %d block - %s. Aborted!", (int)_id, strerror(errno)); \
      res = CR_FAIL; \
      goto _fail; \
   }

..并使用它们编写相同的代码。代码变得更小并且（可以说）更具可读性：

switch(cmd) 
{
case CMD_BLOCK_READ:
   CHECK_KEY(key);
   CHECK_STATE(STATE_BUSY);
   HANDSHAKE(post_resp);
   BLOCK_READ(id, send_nop);
   res = CR_SUCCESS;
   break;
case CMD_BLOCK_WRITE:
   CHECK_KEY(key);
   CHECK_STATE(STATE_BUSY);
   HANDSHAKE(post_resp);
   BLOCK_WRITE(id, send_nop);
   res = CR_SUCCESS;
   break;
case CMD_REQ_START:
{
   CHECK_STATE(STATE_READY);
   state = STATE_BUSY;
   HANDSHAKE(send_nop);
   BLOCK_READ(id, post_resp);
   res = CR_SUCCESS;
   break;
}
/* The remaining 28 or so similar commands */
<..>

代码看起来更像某种脚本语言，而不是好的旧C语言，而且非常丑陋，但为了可读性，我愿意牺牲它。

问题是你如何应对类似的情况？有哪些更优雅的解决方案和最佳实践？

PS我承认，在一般情况下，宏和goto语句是糟糕设计的标志，所以没有必要对它们有多邪恶或我的编程风格有多糟糕而大发雷霆。

score 4 · Accepted Answer

我不会声称 Python 源代码是组织的典范，但它包含（恕我直言）用于简化复杂代码的宏的一个很好的例子。

Python 主循环实现了一个基于堆栈的字节码执行 VM。它包含一个巨大的开关盒，每个 Python 支持的操作码都有一个盒。操作码的调度如下所示：

case STORE_ATTR:
    w = GETITEM(names, oparg);
    v = TOP();
    u = SECOND();
    STACKADJ(-2);
    err = PyObject_SetAttr(v, w, u); /* v.w = u */
    Py_DECREF(v);
    Py_DECREF(u);
    if (err == 0) continue;
    break;

其中TOP和都定义为对堆栈对象进行操作的宏SECOND。一些宏具有用于帮助调试的STACKADJ备用s。#define所有的操作码都是以这种方式编写的，通过这种微型脚本语言表达逻辑，有助于使每个操作码的实现更加清晰。

在我看来，谨慎、明智和有限地使用宏可以提高代码的可读性，并使逻辑更清晰。在您的情况下，宏隐藏了一些小但重要的功能，使用宏来标准化实现并确保您没有要更新的相同代码片段的多个副本可能很有用。

score 1 · Accepted Answer

在这种情况下，我通常会考虑是否可以用数据合理地描述案例，然后在单个公共代码块中处理这些数据。当然不能总是这样做，但通常是可能的。

在您的情况下，它可能会导致类似于以下内容：


#define IO_NOOP    0
#define IO_READ    1
#define IO_WRITE   2

struct cmd_desc { 
   int check_key;     /* non-zero to do a check */
   int check_state;
   int new_state;
   void* handshake_fail;
   int io_dir;
   void* io_fail;
};

const struct cmd_desc cmd_desc_list[] = {
   { 1, STATE_BUSY,  -1,         &&post_resp, IO_READ,  &&send_nop },  /* CMD_BLOCK_READ */
   { 1, STATE_BUSY,  -1,         &&post_resp, IO_WRITE, &&send_nop },  /* CMD_BLOCK_WRITE */
   { 0, STATE_READY, STATE_BUSY, &&send_nop,  IO_READ,  &&post_rep }   /* CMD_REQ_START */
};

const struct cmd_desc* cmd_desc = cmds[cmd];

if(cmd_desc->check_key) {
   if(current_user != key) {
      ERROR("Access violation - invalid key!");
      return CR_ACCESS_DENIED;
   }
}

if(cmd_desc->check_state != -1) {
   if(current_state check_state) {
      WARN("Command %s is not allowed in this state!", cmd_name[cmd]);
      return CR_NOT_PERMITTED;
   }
}

if(cmd_desc->new_state != -1)
   state = cmd_desc->new_state;

switch(cmd_desc->io_dir) {
   case IO_READ:
      if(block_read(id) != 0) {
         ERROR("Failed to read %d block (%s)! Aborted!", id, strerror(errno));
         res = CR_FAIL;
         goto *cmd_desc->io_fail;
      }
      break;

   case IO_WRITE:
      if(block_write(id) != 0) {
         ERROR("Failed to write %d block (%s)! Aborted!", id, strerror(errno));
         res = CR_FAIL;
         goto *cmd_desc->io_fail;
      }
      break;

   case IO_NOOP:
      break;
}

res = CR_SUCCESS;

注释我使用 gcc 的“标签作为值”扩展作为 goto 标签（http://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html）。在标准 C 中，您可能会改用函数指针，但这需要对代码进行一些重组，而我没有足够的信息。

score 1 · Accepted Answer

使用您发布的代码，您没有理由不能使用函数。这将是“提取函数”重构模式。要处理 goto，只需将它们留在主函数中，然后根据函数结果调用或不调用它们。

http://www.refactoring.com/catalog/extractMethod.html

此外，通过在未传入的宏中使用变量，你真的把事情搞得一团糟。这意味着你不能轻易地重用它们，而且它们可能比直接写整个东西更糟糕。如果您传入了宏使用的所有内容，那么它会更有用。然后你会得到一个鸭式风格的编码，可以有效地使用。

此外，您使用的是 C，因此您不应该“避免”宏。它们非常有用，主要用于代码生成。（即字符串化和连接）许多 C++ 和其他一些人说“宏是邪恶的”。这是 C，宏并不邪恶。

c - 在不使用宏的情况下简化并减少代码量

3 回答 3

Related

Reference