Hi,
Le lundi 28 juillet 2008, Johannes Schindelin a écrit :
> Hi,
>
> On Mon, 28 Jul 2008, Christian Couder wrote:
> > + rev = xmalloc((argc - 1) * sizeof(*rev));
> > +
> > + do {
> > + struct commit *r = get_commit_reference(argv[1]);
> > + if (!r)
> > + return 1;
> > + rev[rev_nr++] = r;
> > + argc--; argv++;
> > + } while (argc > 1);
> > +
> > + return show_merge_base(rev, rev_nr, show_all);
>
> rev = xmalloc((argc - 1) * sizeof(*rev));
>
> for (rev_nr = 0; rev_nr + 1 < argc; rev_nr++) {
> rev[rev_nr] = get_commit_reference(argv[rev_nr + 1]);
> if (!rev[rev_nr])
> return !!error("Does not refer to a commit: '%s'",
> argv[rev_nr + 1]);
> }
>
> return show_merge_base(rev, rev_nr, show_all);
>
> I do not know about you, but I think this is not only shorter (in spite
> of adding a helpful error message), but also simpler to understand (not
> using convoluted do { } while logic), and therefore superior.
In my last version the loop is reduced to:
+ do {
+ rev[rev_nr++] = get_commit_reference(argv[1]);
+ argc--; argv++;
+ } while (argc > 1);
so it's very simple.
And the stop condition is simpler in my version.
> Your performance argument is weak IMHO, as this is not a big performance
> hit, and command line parameter parsing is definitely not performance
> critical.
It feels a bit sloppy though.
Regards,
Christian.
--
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
