Code review comment for lp:~ted/indicator-appmenu/hud

>For usage-tracker.c:
>
> Ted did you consider just up-ticking an entry-count, e.g., when it's
> marked, and then returning the count when found? I assume this
> complicates your expiration mechanism. I'm told that the count
> operation is pretty quick so maybe that's fine.

Yeah, I was figuring that not just asking the database was a premature
optimization. I think we can definitely cache that value if it proves
to be something we need. Initially, I was thinking that apps might load
their values on install, but I like the lazy loading mechanism that's
there now so that's no longer an issue. But, yes, performance testing
should guide our decision there.

> I'm assuming it's no coincidence that you gave me a file to review
> which has high test-coverage :) . In configure_db if we get an "Error
> building LRU DB", what happens, and do we really want to continue? I
> see that these are the non-tested paths.

I added checks on the public functions to ensure we have a database. I
think that perhaps we should see if we can't make a file DB, switch to
an in-memory one. Not sure on that, but since it could fail as well, we
need to handle the DB being missing. r185

> Mikkel's advice to add an index is good advice IMHO but I wonder how
> much benefit you'd see as this is a dirt-simple db; it would good to
> add some performance testing for this, e.g.

Yeah, if you look at that branch I made it easier to adjust that. I'm
not sure how to do that performance testing, but I'm open to suggestions
there :-)

> All the GoogleTest work is really excellent to see. This is barely
> worth a commit but consider putting the asserted val first in an
> assertion.

Hmm, I guess my feeling is that "readable" is good here. I'd say
"variable is 5" not "5 is variable". Why do you think the value should
be first? Just to be easier to find?

> On line 362 I'm seeing what might be a 'TODO', but I'm not sure how
> you'd load existing data on a fresh build--what does this mean, and do
> you want to address?

Actually it was before I added the lazy loading of app data scheme.
Removed the comment as it's dated. r186