Design decisions

[Reneg973]

Hi guys,
I'm very new here. I checked the API a bit (just an overview), and here the first question comes into my mind. How did/do you decide which data structure to use for the given problem to solve?

I saw that there are structures/objects used, which produce superfluous overhead. I want to have a lightweight reporting interface to send some telemetry data, putting as little overhead as possible to the current application.
Let me give examples. In Context a handwritten SLL (single linked list) is used, which uses shared_ptrs for its elements. Additionally adding an element appends on the end of the list, requiring to iterate through all elements first.
Interfaces return shared pointers, even if they are not required. Like in most examples a shared pointer is created and than moved into another object. A unique pointer would be more efficient here.

[lalitb]

#828 has some background on the use of shared_ptr within the context. For the thread-local context implementation using the SLL approach, there’s room for optimization - contributions are welcome. The current design might look a bit odd, but it was intentional to keep the context immutable as per the specs, meaning that any addition creates a new context containing all previous elements.

[Reneg973]

Right, especially confusing that the linked list could hold more than one key with the same name. But this second one then will just be found within the other context, therefore ok.
I understood now that the whole list can be shared between contexts. So at the moment I don't see a better solution here. It's just not easily understandable.

My first proposal would be to replace the raw pointer 'key', which could be at least a unique_ptr<[]> or much better, a std::string, giving the advantage of SSO (small string optimisation).
The move assignment of DataList expects that this is empty, and if there is a move assignment, the move constructor is missing.

Edit: I found what confused me most. It's the function "SetValues", which does not what it says. It creates a new context from an existing one. I would've proposed something like:

template<class T>
static Context CreateFrom(const Context &other, const T &values)

[Reneg973]

Hm, checked how the nostd::shared_ptr is implemented, any idea why the shared_ptr_wrapper has virtual functions?

[Reneg973]

Does it really make sense that a ContextValue allows to be monostate? Means one can add keys without values? What is the goal of this? Unfortunately changing this would imply a small API change

[Reneg973]

And what is the goal of all these noexcept keywords? You may want the API to not throw exceptions, that may be ok as a design rule. But then some of the noexcept functions could throw an exception, which is not catched.

From my PoV it would be better to allow exceptions. This opens the possibility to throw at least an invalid_arg in the constructor. This would be good, because currently there is no argument checking, e.g. whether a unique_ptr is nullptr.
Otherwise, it would be required to have member functions like IsValid(), because at the moment some of the classes could be invalid after construction.