Re: [PATCH] remote.c: convert if-else tower to switch

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 7, 2025, 15:02 UTC
Message-ID: <xmqqsei3rx81.fsf@gitster.g>
In-Reply-To: <5866818859be97c091c40602974629eb7e463623.1754540903.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

> For better readability, convert the if-else tower into a switch
> statement.

The reference to "tower" is something new to me. A quick search seems to tell me that "if-else cascade", which is what I've been using around here, is not popular, either. "if-else ladder" is the term more often used, it seems.

Show 8 quoted lines

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> Thanks for the suggestion, both. Please queue this patch wherever it
> makes the most sense to do so (either with the existing series or on its
> own separate branch).
>
>  remote.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

OK. Sitting down and thinking about it, the reason is obvious, but TIL that switch/case is slightly more verbose ;-).

Show 9 quoted lines

> +	case OBJ_BLOB:
>  		advise(_("The <src> part of the refspec is a blob object.\n"
>  			 "Did you mean to tag a new blob by pushing to\n"
>  			 "'%s:refs/tags/%s'?"),
>  		       matched_src_name, dst_value);
> -	} else {
> +		break;
> +	default:
>  		advise(_("The <src> part of the refspec ('%s') is an object ID that doesn't exist.\n"),

This line alone is overly long; it is not part of _this_ patch but is showing the state after that BUG()->advise() fix, so it should be fixed there, I think?

>  		       matched_src_name);
> +		break;
>  	}
>  }