NFD » ndn-cxx

I've been working on a rewritten FaceUri parser in a local branch, as a low priority task. The actual parsing (splitting the input string into URI components) is shared across all schemes, and follows RFC 3986 as closely as possible. After the parsing there is a validation step, which is scheme-specific.

I reviewed https://gerrit.named-data.net/4281 patchset-2.
I like that the parsing steps is cleaner.
I dislike that "validate" functions and "canonize" classes are separate. They should be together. A "scheme handler" (formerly "canonize provider") handles one or more schemes, and does both validation and canonization for these schemes. Each has its own file such as src/net/face-uri-udp.*, and registers itself using a macro similar to NFD's strategy. Shared functions, such as IP address validation, can be placed in base classes.

Junxiao Shi wrote:

I dislike that "validate" functions and "canonize" classes are separate. They should be together.

I haven't touched canonization at all (yet).
Several checks performed by canonization are now redundant with the new parsing algorithm. I plan to remove this duplication. However, a larger refactoring such as the one you propose should be deferred to a separate commit, to make reviewing easier.

Each has its own file such as src/net/face-uri-udp.*, and registers itself using a macro [...]

I'm mildly against splitting into multiple files. The per-scheme logic is a relatively small amount of code.

a larger refactoring such as the one you propose should be deferred to a separate commit, to make reviewing easier.

In contrast, the end result of this refactoring is mostly new code, so I'd rather review them as new code.

I'm mildly against splitting into multiple files. The per-scheme logic is a relatively small amount of code.

CanonizeProvider classes, altogether, are too long. Small files are easier to navigate.