単純なコード ブロックの繰り返しが避けられない場合があります。説明のために、次のコード例を使用します。
注: このコードは説明のみを目的としており、実際のコードはより大きく複雑です。また、エラーが含まれている可能性がありますが、この質問のポイントはそれではありません。
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ステートメントは悪い設計の兆候であることを認めます。したがって、それらがどれほど悪いか、または私のプログラミング スタイルがどれほど貧弱であるかについて炎上する必要はありません。