From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 25 May 2021 08:02:06 GMT
Subject: Re: [PATCH v4 2/4] maintenance: introduce ENABLE/DISABLE for code clarity
Message-ID: <xmqqa6ojfdhd.fsf@gitster.g>
In-Reply-To: <9164987.SKD4A0viWJ@coruscant.lhuard.fr>

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

> 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;".