Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Jan 11, 2012, 04:17 UTC
- Message-ID
- <CALkWK0=bEPPv4rtPrMrQnk3MK=JY4-wwAByWPmzg86NBm_56iQ@mail.gmail.com>
- In-Reply-To
- <CALkWK0k=44znLr2oYSx61Mk=qdAurona0f0H4i4=YXNSAeQhHQ@mail.gmail.com>
Ramkumar Ramachandra wrote:
Show 5 quoted lines
> Fine; I'm sold on the string idea. Also, I figured it would be > easier to explain the changes if I change this enum to a string -- I > should probably use "ease of explaining changes" as a stronger > criterion in the future when I have two competing implementations in > mind.
I wrote that too quickly. I can't stand seeing so many strcmp() calls all over my codebase -- look at the number of instances of matching opts->command to REPLAY_CMD_*. Why should I have to use strcmp() when the data is semantic? It makes no sense: by spelling out the string in so many places, I'm just making the code more error-prone, because the compiler can't warn me if I make a spelling mistake in one place. Why do you like the string so much? A new caller will have to register new actions in the replay_actions enum and modify the codebase to define a codepath for its specific case anyway: so I don't mind it registering a new command in replay_command.
-- Ram