CESNET / libyang-cpp

C++ bindings for the libyang library
https://gerrit.cesnet.cz/q/project:CzechLight/libyang-cpp
BSD 3-Clause "New" or "Revised" License
10 stars 3 forks source link

returning optional<DataNode> from newPath methods #12

Open peckato1 opened 11 months ago

peckato1 commented 11 months ago

Is it neccessary to return optional<DataNode> from newPath methods? Is there any case where the returned node is NULL but the underlying lyd_new_path returns LY_SUCCESS? Also, we always check for std::nullopt in {DataNode,Context}::newPath and throw logic_error if that happens so it seems that we actually might want newPath to return just DataNode.

syyyr commented 11 months ago

Also, we always check for std::nullopt in {DataNode,Context}::newPath

Not 100% true, we only check for std::nullopt in Context::newPath. DataNode::newPath returns std::nullopt if the CreationOptions::Update flag is used and no new nodes were created.

The documentation explains the return value, but doesn't mention the flag:

 * @return Returns the first created node. If no nodes were created, returns std::nullopt.

I agree, that the return value doc for DataNode::newPath should mention the flag. As for Context::newPath, AFAIK, it can't return no new nodes. But I'm not sure, maybe it doesn't if you're creating a non-presence container with the Update flag? I don't know, I'd have to test.