On Fri, Apr 15, 2016 at 10:25 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jacob Keller <jacob.e.keller@xxxxxxxxx> writes: > >> From: Jacob Keller <jacob.keller@xxxxxxxxx> >> >> It is a common pattern in xdl_change_compact to check that hashes and >> strings match. The resulting code to perform this change causes very >> long lines and makes it hard to follow the intention. Introduce a helper >> function xdl_hash_and_recmatch which performs both checks to increase >> code readability. > > Think _why_ it is common to check hash and then do recmatch(). What > is the combination of two trying to compute? > > How about calling it after "what" it computes, not after "how" it > computes it? E.g. > > static int recs_match(xrecord_t **recs, long x, long y, long flags) > > if we answer the above question "they try to see if two records match". > We could also go s/recs/lines/. > > The xdl_recmatch() function appears in xutils.c, and over there the > functions do not use arrays of (xrecord_t *), so I think we are > better off without xdl_ prefix to avoid confusion. > That makes sense. I like the sound of recs_match, and it's shorter too which is nice. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html