Re: [PATCH v7 5/6] refs: allow reference location in refstorage config

[Junio C Hamano <gitster@pobox.com>]

Toon Claes <toon@iotcl.com> writes:

>> +static void parse_reference_uri(const char *value, char **format,
>> +				char **payload)
>> +{
>> +	const char *schema_end;
>> +
>> +	schema_end = strstr(value, "://");
>> +	if (!schema_end) {
>> +		*format = xstrdup(value);
>> +		*payload = NULL;
>> +	} else {
>> +		*format = xstrndup(value, schema_end - value);
>> +		*payload = xstrdup_or_null(schema_end + 3);
>
> Also here, why did you put the negated condition in the if clause?

Hmph, would it make it easier to follow if you swap them?

	if (schema_end) {
		*format = xstrndup(value, schema_end - value);
		*payload = xstrdup_or_null(schema_end + 3);
	} else {
		*format = xstrdup(value);
		*payload = NULL;
	}

Maybe it is just me, but I often find it easier to follow if the
case that require shorter and/or simpler body, or the case that is
narrower (e.g., error condition), comes first before the main logic.
It is in line with preferring an early return on a more specific
condition.  It frees readers from having to worry about these cases
early and let them concentrate on what is expected to usually happen
in the code.

In this particular case, I do not know which one I would prefer,
though.

Thanks.