NFD » ndn-cxx

Davide Pesavento wrote:

we have another append() overload that takes a Component and [...] Component is a subclass of Block

It's actually worse than I thought because Component is implicitly constructible from Block. This means that, after the removal of Name::append(Block), the following code now triggers the implicit conversion from Block to Component and invokes Name::append(Component).

  Block b = ...;
  Name n;
  n.append(b);

This isn't terrible (certainly better than before, at least now the semantics don't depend on the TLV type of b), but I think it's still surprising/unclear to the reader that the append call ends up appending the block as-is (i.e., with whatever TLV type), rather than as a GenericNameComponent (with b becoming the TLV value of that component).
Strictly speaking, this behavior isn't wrong per se, but IMHO it's at the very least:

• inconsistent: other append() overloads either take the TLV-TYPE as an explicit argument, or they wrap the argument in a GenericNameComponent. The "thing" passed as argument is never appended as-is to the name.
• redundant: we already have append(Component) to append as-is, and append(type, value), where value can be anything convertible to span<uint8_t>, to wrap a "thing" in a NameComponent and append it to the name.
• unclear: when reading code using append, it won't be immediately obvious what the code is doing, because overloads that look similar have in fact very different behaviors.

These reasons, coupled with the fact that no other "TLV class" has an implicit constructor from Block, convinced me that we should make the Component(const Block&) constructor explicit.

no.2: https://gerrit.named-data.net/c/ndn-cxx/+/6679
Only ndns is broken.

And after the two previous improvements/cleanups, we can finally introduce an overload of Name::append() that takes a span<> and appends a GenericNameComponent with the given value, without the risk of ambiguous overloads and unclear/surprising semantics.

no.3: https://gerrit.named-data.net/c/ndn-cxx/+/6680