Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>> OK. Or
>> if (!is_fixup(command))
>> BUG("not a FIXUP or SQUASH %d", command);
>> at the very beginning of the function?
>
> Asserting the precondition at the start of the function sounds like a
> good idea
I am of two minds.
I do not know if this is better for longer-term maintainability or
not. Such a message at the beginning of the function declares that
the current implementation does not want to receive a command that
is not either of these two. Such a message tells the next developer
who adds another caller to the function to make sure not to pass
other commands, which is good.
But I do not know what it tells to the next developer who wants to
add support for another command. After opening that initial gate a
bit wider, perhaps to
if (!is_filxup(command) && !is_my_new_command(command))
BUT("not a FIXUP or SQUASH or MYCOMMAND %d", command);
would they make sure to handle their new command correctly in the
if/elseif cascade, from which we are removing the "error()" with
such a change, and would our reviewers notice if they forget to do
so? I dunno.
In any case, such a future change will have to be done with a good
understanding of the entirety of this small function anyway, so I
guess either way is OK.
Thanks.
