Re: [PATCH v4 2/4] maintenance: introduce ENABLE/DISABLE for code clarity

From: Junio C Hamano <gitster@pobox.com>
Date: May 25, 2021, 08:02 UTC
Message-ID: <xmqqa6ojfdhd.fsf@gitster.g>
In-Reply-To: <9164987.SKD4A0viWJ@coruscant.lhuard.fr>

Lénaïc Huard <lenaic@lhuard.fr> writes:

Show 13 quoted lines

> I think the reason why the code looks worse is because I used an enum and I 
> didn’t want to make any assumption about how the enum members would be 
> evaluated in a boolean context.
>
> Do you think it would make sense to drop the enum type, to revert all logic 
> changes (Use `if (enabled)` back instead of `switch`, etc.), and to define the 
> following constants :
>
> static const int DISABLE = 0;
> static const int ENABLE = 1;
>
> so that we can keep function invocation in the form of 
> `launchctl_boot_plist(DISABLE, filename, cmd)` ?

I think the code is much better off without DISABLE/ENABLE at all.

As has already been pointed out, you cannot read and write _without_ being aware of the fact that DISABLE is 0 if you want to write readable code, i.e. instead of "if (able == ENABLE) do this;", you would want to say "if (able) do this;".