NFD

Specific design for pit-entry.hpp as of nfd:commit:5e5e44518286e89a0092ea0ad93c22e64128bdd2 :

class Entry : public StrategyInfoHost, noncopyable
{
public:
  const Interest& getInterest() const;
  const Name& getName() const;
  InRecordCollection inRecords;
  OutRecordCollection outRecords;
public: // misc
  int findNonce(nonce, face) const;
  scheduler::EventId unsatisfyTimer;
  scheduler::EventId stragglerTimer;
};

class InRecordCollection
{
  size_t size() const;
  const_iterator begin() const;
  const_iterator end() const;
  const_iterator find(face) const;
  iterator find(face);
  iterator add(face, interest);
  bool remove(face);
  void clear();
};

class OutRecordCollection
{
  (same as InRecordCollection)
};

• Entry methods under "misc" are not strictly for structure, but they are used by pipelines and it would be inefficient to turn them into StrategyInfo, so they can stay.
• Operations on in-records and out-records are entirely through the collection type. It has been cumbersome to write outRecord = pitEntry->getOutRecord(inFace); if (outRecord == pitEntry->getOutRecords().end()) ... (src) because getOutRecord and end are on different objects; it would look better through the collection type because find and end are on the same object.

canForwardTo violatesScope hasLocalInRecord hasUnexpiredOutRecords should be moved to forwarding as free functions, and included by whatever pipeline or strategy that needs them.

A function would look like: bool violatesScope(const pit::Entry& pitEntry, const Face& inFace).

Junxiao Shi wrote:

Rationale: adding/removing an in-record or out-record not only involves modifications on the collection, but also can change the PIT entry itself.

Just for the record, I don't think that kind of reasoning is correct. The fact that adding an in-record changes the pit entry is an implementation detail, and it should not dictate or influence the API in any way.

I agree with note-4.

Could @Alex approve the API in note-1 then?

Just for the record, I don't think that kind of reasoning is correct. The fact that adding an in-record changes the pit entry is an implementation detail, and it should not dictate or influence the API in any way.

I disagree with this. The semantics of PIT entry is not just a collection of in and out interests. It is a state that remembers in and out, which is being update whenever you add/remove in/out interests. Yes, it can be separated, but we then we simply moving the logic to somewhere and potentially "violate" PIT entry semantics.

Maintaining a class or object invariant is also an implementation detail. Of course the API must not provide a way for its consumer to break the invariant. I wasn't suggesting to do that.

I think the question is whether a pit entry is (from an API point of view) an atomic, stateful entity that appears as a black box, or an aggregate of inRecords + outRecords + some other state. I have no strong opinion on this. All I'm saying is that the rationale in note-3 should not --by itself-- rule out the second definition of pit entry.

whether a pit entry is an atomic, stateful entity that appears as a black box, or an aggregate of inRecords + outRecords + some other state

Conceptually, a PIT entry should be an atomic entity, because an in-record or out-record is meaningless without a PIT entry.

However, I'm unsure how this question affects API design.

Junxiao Shi wrote:

However, I'm unsure how this question affects API design.

It doesn't have to. What a pit entry conceptually is doesn't necessarily shape every aspect of its API. It can certainly influence it, but some invariants can be maintained even if the "API model" is different from the "conceptual model". That's why I said "from an API point of view" in note-7.

It doesn't have to. What a pit entry conceptually is doesn't necessarily shape every aspect of its API.

In this case, I'll stick with note-3 design which has been approved.

http://gerrit.named-data.net/2787

patchset1 moves canForwardTo violatesScope findNonce to forwarding.

I'll do API refactoring (according to note-3) in next patchset.

I'm waiting for a decision on #3545 before moving hasUnexpiredOutRecords.

This task is easier than expected.

http://gerrit.named-data.net/2806

This commit deduplicates Forwarder::LOCALHOST_NAME and scope_prefix::LOCALHOST.