git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Hariom Verma" <hariom18599@gmail.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Philip Oakley" <philipoakley@iee.email>,
	"ZheNing Hu" <adlternative@gmail.com>,
	"ZheNing Hu" <adlternative@gmail.com>
Subject: [PATCH 8/8] [GSOC] ref-filter: use switch/case instead of if/else
Date: Tue, 17 Aug 2021 08:41:41 +0000	[thread overview]
Message-ID: <2321b873d0c0223e553492d80ced2a51d8ce7281.1629189701.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1021.git.1629189701.gitgitgadget@gmail.com>

From: ZheNing Hu <adlternative@gmail.com>

In the logic of ref-filter, if/else is used extensively
to check the atom type. This is because before we introduce
atom_type, the atom check is achieved through string comparison.

Using switch/case can allow the compiler to reduce unnecessary
branch checks and increase the readability of the code.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 163 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 109 insertions(+), 54 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index f1c82e20e3d..c0b5d30ac57 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1110,9 +1110,11 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
 			continue;
-		if (atom_type == ATOM_OBJECTTYPE)
+		switch (atom_type) {
+		case ATOM_OBJECTTYPE : {
 			v->s = xstrdup(type_name(oi->type));
-		else if (atom_type == ATOM_OBJECTSIZE) {
+			break;
+		} case ATOM_OBJECTSIZE : {
 			if (used_atom[i].u.objectsize.option == O_SIZE_DISK) {
 				v->value = oi->disk_size;
 				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX, (uintmax_t)oi->disk_size);
@@ -1120,10 +1122,20 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 				v->value = oi->size;
 				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX , (uintmax_t)oi->size);
 			}
-		} else if (atom_type == ATOM_DELTABASE)
+			break;
+		}
+		case ATOM_DELTABASE: {
 			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
-		else if (atom_type == ATOM_OBJECTNAME && deref) {
-			v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i]));
+			break;
+		}
+		case ATOM_OBJECTNAME: {
+			if (deref)
+				v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i]));
+			break;
+		}
+		default: {
+			break;
+		}
 		}
 	}
 }
@@ -1138,13 +1150,22 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
-			continue;
-		if (atom_type == ATOM_TAG)
+			break;
+		switch (atom_type) {
+		case ATOM_TAG:
 			v->s = xstrdup(tag->tag);
-		else if (atom_type == ATOM_TYPE && tag->tagged)
-			v->s = xstrdup(type_name(tag->tagged->type));
-		else if (atom_type == ATOM_OBJECT && tag->tagged)
-			v->s = xstrdup(oid_to_hex(&tag->tagged->oid));
+			break;
+		case ATOM_TYPE:
+			if (tag->tagged)
+				v->s = xstrdup(type_name(tag->tagged->type));
+			break;
+		case ATOM_OBJECT:
+			if (tag->tagged)
+				v->s = xstrdup(oid_to_hex(&tag->tagged->oid));
+			break;
+		default:
+			break;
+		}
 	}
 }
 
@@ -1159,15 +1180,17 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
 			continue;
-		if (atom_type == ATOM_TREE) {
+		switch (atom_type) {
+		case ATOM_TREE: {
 			v->s = xstrdup(do_grab_oid("tree", get_commit_tree_oid(commit), &used_atom[i]));
-			continue;
+			break;
 		}
-		if (atom_type == ATOM_NUMPARENT) {
+		case ATOM_NUMPARENT: {
 			v->value = commit_list_count(commit->parents);
 			v->s = xstrfmt_len(&v->s_size, "%lu", (unsigned long)v->value);
+			break;
 		}
-		else if (atom_type == ATOM_PARENT) {
+		case ATOM_PARENT: {
 			struct commit_list *parents;
 			struct strbuf s = STRBUF_INIT;
 			for (parents = commit->parents; parents; parents = parents->next) {
@@ -1177,6 +1200,11 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 				strbuf_addstr(&s, do_grab_oid("parent", oid, &used_atom[i]));
 			}
 			v->s = strbuf_detach(&s, NULL);
+			break;
+		}
+		default: {
+			break;
+		}
 		}
 	}
 }
@@ -1843,24 +1871,40 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		v->handler = append_atom;
 		v->atom = atom;
 
-		if (atom_type == ATOM_REFNAME)
+		switch (atom_type) {
+		case ATOM_REFNAME: {
 			refname = get_refname(atom, ref);
-		else if (atom_type == ATOM_WORKTREEPATH) {
+			if (!deref)
+				v->s = xstrdup(refname);
+			else
+				v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
+			free((char *)refname);
+			break;
+		}
+		case ATOM_WORKTREEPATH: {
 			if (ref->kind == FILTER_REFS_BRANCHES)
 				v->s = get_worktree_path(atom, ref);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
+			break;
 		}
-		else if (atom_type == ATOM_SYMREF)
+		case ATOM_SYMREF: {
 			refname = get_symref(atom, ref);
-		else if (atom_type == ATOM_UPSTREAM) {
+			if (!deref)
+				v->s = xstrdup(refname);
+			else
+				v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
+			if (refname != ref_filter_slopbuf)
+				free((char *)refname);
+			break;
+		}
+		case ATOM_UPSTREAM: {
 			const char *branch_name;
 			/* only local branches may have an upstream */
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name)) {
 				v->s = ref_filter_slopbuf;
-				continue;
+			break;
 			}
 			branch = branch_get(branch_name);
 
@@ -1869,13 +1913,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				fill_remote_ref_details(atom, refname, branch, v);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {
+			break;
+		}
+		case ATOM_PUSH: {
 			const char *branch_name;
+
+			if (!atom->u.remote_ref.push)
+				break;
 			v->s = ref_filter_slopbuf;
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name))
-				continue;
+				break;
 			branch = branch_get(branch_name);
 
 			if (atom->u.remote_ref.push_remote)
@@ -1883,14 +1931,16 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			else {
 				refname = branch_get_push(branch, NULL);
 				if (!refname)
-					continue;
+					break;
 			}
 			fill_remote_ref_details(atom, refname, branch, v);
-			continue;
-		} else if (atom_type == ATOM_COLOR) {
+			break;
+		}
+		case ATOM_COLOR: {
 			v->s = xstrdup(atom->u.color);
 			continue;
-		} else if (atom_type == ATOM_FLAG) {
+		}
+		case ATOM_FLAG: {
 			char buf[256], *cp = buf;
 			if (ref->flag & REF_ISSYMREF)
 				cp = copy_advance(cp, ",symref");
@@ -1902,54 +1952,59 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				*cp = '\0';
 				v->s = xstrdup(buf + 1);
 			}
-			continue;
-		} else if (!deref && atom_type == ATOM_OBJECTNAME) {
-			   v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom));
-			   continue;
-		} else if (atom_type == ATOM_HEAD) {
+			break;
+		}
+		case ATOM_OBJECTNAME: {
+			if (!deref)
+				v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom));
+			break;
+		}
+		case ATOM_HEAD: {
 			if (atom->u.head && !strcmp(ref->refname, atom->u.head))
 				v->s = xstrdup("*");
 			else
 				v->s = xstrdup(" ");
-			continue;
-		} else if (atom_type == ATOM_ALIGN) {
+			break;
+		}
+		case ATOM_ALIGN: {
 			v->handler = align_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_END) {
+			break;
+		}
+		case ATOM_END: {
 			v->handler = end_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_IF) {
+			break;
+		}
+		case ATOM_IF: {
 			const char *s;
 			if (skip_prefix(name, "if:", &s))
 				v->s = xstrdup(s);
 			else
 				v->s = ref_filter_slopbuf;
 			v->handler = if_atom_handler;
-			continue;
-		} else if (atom_type == ATOM_THEN) {
+			break;
+		}
+		case ATOM_THEN: {
 			v->handler = then_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_ELSE) {
+			break;
+		}
+		case ATOM_ELSE: {
 			v->handler = else_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_REST) {
+			break;
+		}
+		case ATOM_REST: {
 			if (ref->rest)
 				v->s = xstrdup(ref->rest);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
-		} else
-			continue;
-
-		if (!deref)
-			v->s = xstrdup(refname);
-		else
-			v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
-		free((char *)refname);
+			break;
+		}
+		default:
+			break;
+		}
 	}
 
 	for (i = 0; i < used_atom_cnt; i++) {
-- 
gitgitgadget

      parent reply	other threads:[~2021-08-17  8:41 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17  8:41 [PATCH 0/8] [GSOC] [RFC] ref-filter: code logic optimization ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 1/8] [GSOC] ref-filter: remove grab_oid() function ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 2/8] [GSOC] ref-filter: merge two for loop in grab_person ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 3/8] [GSOC] ref-filter: remove strlen from find_subpos ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 4/8] [GSOC] ref-filter: introducing xstrvfmt_len() and xstrfmt_len() ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 5/8] [GSOC] ref-filter: introduction ref_filter_slopbuf[1] ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 6/8] [GSOC] ref-filter: add deref member to struct used_atom ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` [PATCH 7/8] [GSOC] ref-filter: introduce symref_atom_parser() ZheNing Hu via GitGitGadget
2021-08-17  8:41 ` ZheNing Hu via GitGitGadget [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2321b873d0c0223e553492d80ced2a51d8ce7281.1629189701.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=adlternative@gmail.com \
    --cc=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hariom18599@gmail.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.email \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).