Re: [PATCH 12/13] remote.c: refactor get_remote_ref_states()

On Mon, Feb 23, 2009 at 1:50 AM, Jeff King [off-list ref] wrote:

On Mon, Feb 23, 2009 at 01:29:00AM -0500, Jay Soffian wrote:

quoted

get_remote_ref_states() has three callers, but each is interested in
slightly different information. Give it a bit-field flag so that callers
can specify which pieces of information they need.

Hmph. I think this is probably an indication that
get_remote_ref_states() should really be 3 functions:

 common_stuff();
 query_one();
 query_two();

and then callers can choose the subset they are interested in.

Which is really more or less equivalent; it just seems like extra
obfuscation to have a single function with a bit-field.

I see your hmph and raise you a hmph. :-)

Well, I _had_ tried as you suggested first, and thought it yuckier. It
would actually be more like:

caller1() {
  setup_for_get();
  get_thing_one();
}

caller2() {
  setup_for_get();
  get_thing_two();
}

caller3() {
  if (query) {
     setup_for_get();
     get_thing_one();
     get_thing_two();
     get_thing_three();
} else {
     get_thing_one_noquery()
     get_thing_two_noquery();
}

As opposed to:

caller1() {
  get_things(ONE)
}

caller2() {
  get_things(TWO)
}

caller3() {
  things = 0;
  if (query)
     things = (ONE|TWO|THREE)
  get_things(things)
}

I'm not sure why passing a flag saying what you want is obfuscating.
Also, I did find other places in git that do this sort of thing (e.g.,
get_remote_heads).

(Awaiting Junio's lucid reply explaining why my justification is bogus.) :-)

j.