NFD

https://gerrit.named-data.net/3649 introduces a temporary workaround that GenericLinkServiceCounters inheritance is no longer virtual.
This improves the situation so that GenericLinkService can be inherited from, but the subclass would not be able to use a different Counters type.
This workaround should be reverted when this issue is resolved.

I will start work on this issue after #3931 is merged, as it also modifies GenericLinkService.

https://gerrit.named-data.net/3817 makes GenericLinkService::m_reassembler protected, so that GenericLinkService can be inherited from. However, this still requires every subclass to be aware of this and to call GenericLinkServiceCounters constructor directly. Moreover, if the same pattern is used, every new parameter of GenericLinkServiceCounters constructor would trigger a widespread change.
Is it possible to avoid this constructor parameter, so that subclass doesn't need to do anything?

I'm thinking about:

• LpReassembler provides its counters in LpReassemblerCounters class.
• GenericLinkServiceCounters inherits from LpReassemblerCounters.
• LpReassembler constructor takes LpReassemblerCounters& parameter.

Junxiao Shi wrote:

https://gerrit.named-data.net/3817 makes GenericLinkService::m_reassembler protected, so that GenericLinkService can be inherited from. However, this still requires every subclass to be aware of this and to call GenericLinkServiceCounters constructor directly. Moreover, if the same pattern is used, every new parameter of GenericLinkServiceCounters constructor would trigger a widespread change.

I also dislike that solution.

• LpReassembler provides its counters in LpReassemblerCounters class.
• GenericLinkServiceCounters inherits from LpReassemblerCounters.
• LpReassembler constructor takes LpReassemblerCounters& parameter.

I don't see how that would help... LpReassemblerCounters constructor would still require a const LpReassembler& parameter, no?

I don't see how that would help... LpReassemblerCounters constructor would still require a const LpReassembler& parameter, no?

No.

LpReassembler constructor takes LpReassemblerCounters& parameter.

The counter variable is in LpReassemblerCounters, and LpReassembler stores a reference to LpReassemblerCounters. SizeCounter cannot be used, but LpReassembler has to manually increment and decrement the counter like ++m_counters.nReassembling when appropriate.

Junxiao Shi wrote:

The counter variable is in LpReassemblerCounters, and LpReassembler stores a reference to LpReassemblerCounters. SizeCounter cannot be used, but LpReassembler has to manually increment and decrement the counter like ++m_counters.nReassembling when appropriate.

Well, this changes everything. In particular, the critical change is to stop using SizeCounter, that's what solves the constructor issue, not the changes in note-7.

I don't like this solution either though, it feels more like a workaround than a proper solution.

How about:

• SizeCounter is changed to observe a const T* instead of a const T&
• SizeCounter removes its constructor parameter, or defaults it to nullptr, so that it can be default constructed, and in addition it gains a setObservedObject(const T*) member function
• Either GenericLinkService or LpReassembler constructor calls setObservedObject on the nReassembling counter

I agree with note-10 design.