Re: [PATCH 09/19] reset.c: replace switch by if-else

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2013, 19:53 UTC
Message-ID: <7vhamq5e1v.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1357719376-16406-10-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

Show 27 quoted lines

> ---
>  builtin/reset.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 42d1563..05ccfd4 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	 * saving the previous head in ORIG_HEAD before. */
>  	update_ref_status = update_refs(rev, sha1);
>  
> -	switch (reset_type) {
> -	case HARD:
> -		if (!update_ref_status && !quiet)
> -			print_new_head_line(commit);
> -		break;
> -	case SOFT: /* Nothing else to do. */
> -		break;
> -	case MIXED: /* Report what has not been updated. */
> +	if (reset_type == HARD && !update_ref_status && !quiet)
> +		print_new_head_line(commit);
> +	else if (reset_type == MIXED) /* Report what has not been updated. */
>  		update_index_refresh(0, NULL,
>  				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
> -		break;
> -	}

Justification?

It might be shorter, but I somehow find the original _much_ easier to follow, and to possibly extend. The case arms delineate the major modes of operation, and when somebody is interested in what happens in "reset --hard", the case labels allow eyes to immediately spot and skip uninteresting case arms. On the other hand, the updated one forces you to read the if/else cascade through.