From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Wed, 11 Jan 2012 04:17:51 GMT
Subject: Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
Message-ID: <CALkWK0=bEPPv4rtPrMrQnk3MK=JY4-wwAByWPmzg86NBm_56iQ@mail.gmail.com>
In-Reply-To: <CALkWK0k=44znLr2oYSx61Mk=qdAurona0f0H4i4=YXNSAeQhHQ@mail.gmail.com>

Ramkumar Ramachandra wrote:
> 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