Re: [PATCH v2 11/15] git_config_set: do not use a state machine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 03, 2018 at 06:28:42PM +0200, Johannes Schindelin wrote:

> While a neat theoretical construct, state machines are hard to read. In
> this instance, it does not even make a whole lot of sense because we are
> more interested in flags, anyway: has the section been seen? Has the key
> been seen? Does the current section match the key we are looking for?
> 
> Besides, the state `SECTION_SEEN` was named in a misleading way: it did
> not indicate that we saw the section matching the key we are looking
> for, but it instead indicated that we are *currently* in that section.
> 
> Let's just replace the state machine logic by clear and obvious flags.
> 
> This will also make it easier to review the upcoming patches to use the
> newly-introduced `event_fn` callback of the config parser.

I think this is probably a good direction. But one thing state machines
can help with is keeping the state to a manageable size. With 3 bits of
flags, we now have 8 possible states, up from the previous 4.

Clearly some of those are nonsensical (can you be in key_seen without
section_seen? I'd think not), but it's up to the code to interpret and
reset those manually.

I'll defer to your judgement, though, on this making things for the
future patches more readable. You spend a lot more time poking at it
than I have.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux