Skip to main content
deleted 18 characters in body
Source Link

If you separated the three cases because the opcode values/ranges mean different things, and you therefore chose the same action (accept/skip) for different reasons, I think your style is fine. A comment describing the meaning of each case is useful for the next person to read the code:

if (opcode == OP_0) { /* OP_0 (zero) is a no-op, ok */}
    else if (opcode == OP_1NEGATE) { /* flip register value */}
    else if (opcode >= OP_1 && opcode <= OP_16) { /* known allocated opcodes */}
    else { throw new Exception("decodeFromOpN called on non OP_N opcode"); }

This way it's clearer that the cases had different meanings when you wrote it. As the code evolves, future maintainers can add/further breakbreak out/consolidate cases as their meaning changes, and/or add debugging information or change actions for the individual cases if necessary.

However, per others' answers, if the cases have roughly the same meaning in the local context of this code, consolidating the conditions makes sense.

If you separated the three cases because the opcode values/ranges mean different things, and you therefore chose the same action (accept/skip) for different reasons, I think your style is fine. A comment describing the meaning of each case is useful for the next person to read the code:

if (opcode == OP_0) { /* OP_0 (zero) is a no-op, ok */}
    else if (opcode == OP_1NEGATE) { /* flip register value */}
    else if (opcode >= OP_1 && opcode <= OP_16) { /* known allocated opcodes */}
    else { throw new Exception("decodeFromOpN called on non OP_N opcode"); }

This way it's clearer that the cases had different meanings when you wrote it. As the code evolves, future maintainers can add/further break out/consolidate cases as their meaning changes, and/or add debugging information or change actions for the individual cases if necessary.

However, per others' answers, if the cases have roughly the same meaning in the local context of this code, consolidating the conditions makes sense.

If you separated the three cases because the opcode values/ranges mean different things, and you chose the same action (accept/skip) for different reasons, I think your style is fine. A comment describing the meaning of each case is useful for the next person to read the code:

if (opcode == OP_0) { /* OP_0 (zero) is a no-op, ok */}
    else if (opcode == OP_1NEGATE) { /* flip register value */}
    else if (opcode >= OP_1 && opcode <= OP_16) { /* known allocated opcodes */}
    else { throw new Exception("decodeFromOpN called on non OP_N opcode"); }

This way it's clearer that the cases had different meanings when you wrote it. As the code evolves, future maintainers can add/break out/consolidate cases as their meaning changes, and/or add debugging information or change actions for the individual cases if necessary.

However, per others' answers, if the cases have roughly the same meaning in the local context of this code, consolidating the conditions makes sense.

Source Link

If you separated the three cases because the opcode values/ranges mean different things, and you therefore chose the same action (accept/skip) for different reasons, I think your style is fine. A comment describing the meaning of each case is useful for the next person to read the code:

if (opcode == OP_0) { /* OP_0 (zero) is a no-op, ok */}
    else if (opcode == OP_1NEGATE) { /* flip register value */}
    else if (opcode >= OP_1 && opcode <= OP_16) { /* known allocated opcodes */}
    else { throw new Exception("decodeFromOpN called on non OP_N opcode"); }

This way it's clearer that the cases had different meanings when you wrote it. As the code evolves, future maintainers can add/further break out/consolidate cases as their meaning changes, and/or add debugging information or change actions for the individual cases if necessary.

However, per others' answers, if the cases have roughly the same meaning in the local context of this code, consolidating the conditions makes sense.