Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> + if ((flags & LOOKUP_LITERALLY)) {
> + if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)
> + status = error("unable to unpack %s header with --literally",
> + sha1_to_hex(sha1));
> + else if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
> + status = error("unable to parse %s header", sha1_to_hex(sha1));
> + } else {
> + if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
> + status = error("unable to unpack %s header",
> + sha1_to_hex(sha1));
> + else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
> + status = error("unable to parse %s header", sha1_to_hex(sha1));
> + }
I wonder if you can further reduce an unnecessary duplication in the
two "else if" clauses in the above, and if the result becomes easier
to read and maintain. Perhaps
if (((flags & LOOKUP_LITERALLY)
? unpack_sha1_header_to_strbuf(...)
: unpack_sha1_header(...)) < 0)
status = error(...);
else if ((status = parse_sha1_header_extended(...)) < 0)
status = error(...);
or even
status = 0;
if (flags & LOOKUP_LITERALLY) {
if (unpack_sha1_header_to_strbuf(...) < 0)
status = error(...);
} else {
if (unpack_sha1_header(...) < 0)
status = error(...);
}
if (!status) {
if (status = parse(...)) < 0)
status = error(...);
}
although I think the latter might be a bit harder to read.
--
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