Hi az,
I'd be tempted to make it an if-then with no else clause by hoisting
the "BCC:" prefix and "\n" suffix outside of the if-then.
hmm. i see your point, but don't entirely agree. my aim here was to
contain all the related logic within the smallest possible/sensible
horizon.
apart from the small ugliness of having the string "bcc:" hardcoded
twice i prefer that 'if' block doing its thing (and all of its thing!)
over strings conditionally accumulating across a pageful of code or
more.
I don't quite get the comparison so think I may not have got my idea
over. I don't know the context of the patch's chunk so considered it in
isolation. The original,
for (lp = localaddrs.m_next; lp; lp = lp->m_next)
if (lp->m_bcc)
allbcc = allbcc? add(concat(", ", lp->m_mbox, NULL), allbcc)
: mh_xstrdup(lp->m_mbox);
for (lp = netaddrs.m_next; lp; lp = lp->m_next)
if (lp->m_bcc)
allbcc = allbcc? add(
concat(", ", lp->m_mbox, "@", lp->m_host, NULL),
allbcc)
: concat(lp->m_mbox, "@", lp->m_host, NULL);
if (allbcc)
{
fprintf (out, "BCC: %s\n",allbcc);
free(allbcc);
}
seems simpler as
for (lp = localaddrs.m_next; lp; lp = lp->m_next)
if (lp->m_bcc)
allbcc = add(concat(", ", lp->m_mbox, NULL), allbcc);
for (lp = netaddrs.m_next; lp; lp = lp->m_next)
if (lp->m_bcc)
allbcc = add(concat(", ", lp->m_mbox, "@", lp->m_host, NULL),
allbcc);
if (allbcc) {
fprintf(out, "BCC: %s\n", allbcc + 1);
free(allbcc);
}
though I haven't run it, let alone tested it, so it could be simpler but
wrong. :-)
anyway, these are my personal preferences; i hope we can agree to
disagree and that somebody with commit rights will apply the patch or
equivalent to the code.
Yep, absolutely. I see it's already getting attention from David and
I expect Ken will look too.
--
Cheers, Ralph.
--
nmh-workers
https://lists.nongnu.org/mailman/listinfo/nmh-workers