[pacman-dev] Long functions and other minor fixes

[Aaron Griffin]

On 12/28/06, Dan McGee <dpmcgee at gmail.com> wrote:
> This is a refactoring for _alpm_sync_sysupgrade, but I didn't really
> cut down on the total code length. I just thought the triple nested
> for loops deserved their own function (and maybe this will lead to
> inspiration to cleaning up this code). The patch looks kind of weird
> but a vimdiff makes a lot more sense of it.

This is great! Exactly what I'm looking to do.  I've applied it, but
made a few minor changes (while I like goto in some cases, it can be
factored out in the above code, so I did so).

Another important rule of thumb was best said by Linus himself:
   If you need more than three levels of
   indentation, you're screwed anyway, and
   should fix your program.

All gargantuan nested things are just... too much. It's too hard to
follow, and this prevents people from contributing.

For those interested parties who don't know what I'm talking about,
here's an example of how something like this would work (though not
really in the spirit of what linus is saying):

for(int i...) {
   for(int j...) {
      for(int k...) {
         for(int l...) {
         }
      }
   }
}

can become:
void do_something(int i, int j) {
   for(int k...) {
      for(int l...) {
      }
   }
}

for(int i...) {
   for(int j...) {
      do_something(i, j);
   }
}


While this is not EXACTLY what the "three levels of indentation" adage
is saying, this is a start to refactoring some of this stuff.

On a side note, alot of these functions do things like
"find_package(db, pkgname)" in a loop over all DBs.  To me, it makes
much more sense to just do "find_package(pkgname)" and let that
function find the appropriate DB.  The package structure does actually
contain a pointer to the DB (if it's a sync package... kinda nasty,
but it works well enough), so we can back-reference the DB once it's
found if we need to.  Alot of these (db, pkg) functions can be cleaned
up in this case, assuming there are proper checks to verify we're
looking at a sync package.

So, for anyone interested, I still have a bunch of bugtracker bugs (17
at my last count, excluding FRs), and need to get those together, but
I will be working on refactoring some of this stuff as I touch things
too.  If anyone is interested in doing some code cleanup (yeah, it's a
thankless job) let me know.  I'd like to organize this so we don't
step on each other's toes.  Also, feel free to check out the bug
reports.  There appears to be 3-4 that have to do with dep tracking.