Re: [PATCH 1/5] builtin/repo: update stats for each object

> Justin Tobler <jltobler@gmail.com> writes:
> 
> > +		switch (type) {
> > +		case OBJ_TAG:
> > +			stats->type_counts.tags++;
> > +			stats->inflated_sizes.tags += inflated;
> > +			stats->disk_sizes.tags += disk;
> > +			break;
> > +		case OBJ_COMMIT:
> > +			stats->type_counts.commits++;
> > +			stats->inflated_sizes.commits += inflated;
> > +			stats->disk_sizes.commits += disk;
> > +			break;
> > +		case OBJ_TREE:
> > +			stats->type_counts.trees++;
> > +			stats->inflated_sizes.trees += inflated;
> > +			stats->disk_sizes.trees += disk;
> > +			break;
> > +		case OBJ_BLOB:
> > +			stats->type_counts.blobs++;
> > +			stats->inflated_sizes.blobs += inflated;
> > +			stats->disk_sizes.blobs += disk;
> > +			break;
> > +		default:
> > +			BUG("invalid object type");
> > +		}
> >  	}
> 
> The repetition above makes me wonder if it might be a better
> organization to have
> 
>     struct object_stat {       
>         struct type_stat {
>             size_t count;
>             size_t inflated_size;
>             size_t disk_size;
> 	} tag, commit, tree, blob;
> 	... possibly other members ...
>     } *stats;
> 
> or even
> 
>     struct object_stat {       
>         struct type_stat {
>             size_t count;
>             size_t inflated_size;
>             size_t disk_size;
> 	} t[4];
> 	... possibly other members ...
>     };
> 
> and have this part of the code be
> 
> 	struct type_stat *t;
> 
> 	if (OBJ_COMMIT <= type && type <= OBJ_TAG)
> 		t = stats->t[type - 1];
> 	else
> 		BUG("invalid object type");
> 
> 	t->count++;
> 	t->inflated_size += inflated;
> 	t->disk_size += disk;
> 
> but that is probably only because I am looking at this part of the
> code.  Other parts of the code may have good reasons to have the
> structure nested the other way around like you have.