diff --git a/include/xtrpg/xml/node/DeclarationNode.hpp b/include/xtrpg/xml/node/DeclarationNode.hpp index 23038d9..14d4069 100644 --- a/include/xtrpg/xml/node/DeclarationNode.hpp +++ b/include/xtrpg/xml/node/DeclarationNode.hpp @@ -27,9 +27,9 @@ class DeclarationNode : public INode, public ITagname, public IAttributes { } /** - * Explicitly defaulted copy constructor. + * Deleted copy constructor - move-only semantics for consistency and safety. */ - DeclarationNode(const DeclarationNode &) = default; + DeclarationNode(const DeclarationNode &) = delete; /** * Explicitly defaulted move constructor. @@ -37,9 +37,10 @@ class DeclarationNode : public INode, public ITagname, public IAttributes { DeclarationNode(DeclarationNode &&) = default; /** - * Explicitly defaulted copy assignment operator. + * Deleted copy assignment operator - move-only semantics for consistency and + * safety. */ - DeclarationNode &operator=(const DeclarationNode &) = default; + DeclarationNode &operator=(const DeclarationNode &) = delete; /** * Explicitly defaulted move assignment operator. diff --git a/include/xtrpg/xml/node/NodeContainer.hpp b/include/xtrpg/xml/node/NodeContainer.hpp index 599e276..0ddbefe 100644 --- a/include/xtrpg/xml/node/NodeContainer.hpp +++ b/include/xtrpg/xml/node/NodeContainer.hpp @@ -18,36 +18,69 @@ class NodeContainer : public INode { explicit NodeContainer(NodeType type) : INode(type) {} - ~NodeContainer() = default; + // Delete copy semantics - ownership transfer is explicit via move + NodeContainer(const NodeContainer &) = delete; + NodeContainer &operator=(const NodeContainer &) = delete; + + // Keep move semantics to allow ownership transfer + NodeContainer(NodeContainer &&) = default; + NodeContainer &operator=(NodeContainer &&) = default; + + ~NodeContainer() { + for (INode *child : _children) { + delete child; + } + this->_children.clear(); + }; /** * Appends a given child node. * * @throws std::invalid_argument if appending would create a cycle. */ - void append(std::shared_ptr child) { - if (child) { - // Check if this node is already an ancestor of child by walking up - // the parent chain. This is O(depth) instead of O(n). - for (NodeContainer *ancestor = child->getParent(); ancestor != nullptr; - ancestor = ancestor->getParent()) { - if (ancestor == this) { - throw std::invalid_argument("Cannot create a cycle in XML nodes"); - } + void append(INode *ptrChild) { + if (nullptr == ptrChild) { + return; + } + + // Check if this node is already an ancestor of child by walking up + // the parent chain. This is O(depth) instead of O(n). + for (NodeContainer *ancestor = ptrChild->getParent(); ancestor != nullptr; + ancestor = ancestor->getParent()) { + if (ancestor == this) { + throw std::invalid_argument("Cannot create a cycle in XML nodes"); } - // Set this node as the child's parent and append - child->setParent(this); - this->_children.push_back(std::move(child)); } + // Set this node as the child's parent and append + ptrChild->setParent(this); + this->_children.push_back(ptrChild); } /** - * Returns a vector of child nodes. + * Appends a new TextNode containing the provided string to this container. + * Exception-safe: if append fails, the TextNode is cleaned up before + * re-throwing the exception. + * + * @param withText the text content for the new TextNode + * @throws std::invalid_argument if the text contains invalid XML characters + * or if appending would create a cycle (though cycles are not + * possible with newly created TextNodes). */ - const std::vector> &children() const { - return this->_children; + void append(const std::string &withText) { + TextNode *_ptrNode = new TextNode(withText); + try { + this->append(_ptrNode); + } catch (...) { + delete _ptrNode; + throw; + } } + /** + * Returns a vector of child nodes. + */ + const std::vector &children() const { return this->_children; } + /** * Checks whether this node has any child nodes. * @@ -67,15 +100,14 @@ class NodeContainer : public INode { } private: - std::vector> _children; + std::vector _children; }; /** * Stream operator overload for easy serialization */ -inline NodeContainer &operator<<(NodeContainer &node, - std::shared_ptr child) { - node.append(std::move(child)); +inline NodeContainer &operator<<(NodeContainer &node, INode *child) { + node.append(child); return node; } @@ -85,8 +117,13 @@ inline NodeContainer &operator<<(NodeContainer &node, */ inline NodeContainer &operator<<(NodeContainer &node, const std::string &withText) { - auto textNode = std::make_shared(withText); - node.append(textNode); + TextNode *_ptrNode = new TextNode(withText); + try { + node.append(_ptrNode); + } catch (...) { + delete _ptrNode; + throw; + } return node; } } // namespace xtrpg::xml::node \ No newline at end of file diff --git a/include/xtrpg/xml/node/TagNode.hpp b/include/xtrpg/xml/node/TagNode.hpp index d38d0ea..8728f0f 100644 --- a/include/xtrpg/xml/node/TagNode.hpp +++ b/include/xtrpg/xml/node/TagNode.hpp @@ -31,9 +31,10 @@ class TagNode : public ITagname, public IAttributes, public NodeContainer { : ITagname(name), IAttributes(), NodeContainer(NodeType::TAG) {} /** - * Explicitly defaulted copy constructor. + * Deleted copy constructor - prevents accidental shallow copies of child + * nodes. Use move semantics for explicit ownership transfer. */ - TagNode(const TagNode &) = default; + TagNode(const TagNode &) = delete; /** * Explicitly defaulted move constructor. @@ -41,9 +42,10 @@ class TagNode : public ITagname, public IAttributes, public NodeContainer { TagNode(TagNode &&) = default; /** - * Explicitly defaulted copy assignment operator. + * Deleted copy assignment operator - prevents accidental shallow copies of + * child nodes. Use move semantics for explicit ownership transfer. */ - TagNode &operator=(const TagNode &) = default; + TagNode &operator=(const TagNode &) = delete; /** * Explicitly defaulted move assignment operator. @@ -55,6 +57,41 @@ class TagNode : public ITagname, public IAttributes, public NodeContainer { */ const std::string_view name() const { return this->getTagname(); } + /** + * Appends a new TagNode with the provided tag name to this container. + * The consumer function is called with the new TagNode to allow configuration + * before it is appended. This pattern enables fluent, nested construction of + * XML trees. + * + * Exception-safe: if the consumer throws or append fails, the TagNode is + * cleaned up before re-throwing the exception. + * + * @param tagname the name for the new TagNode + * @param consumer a callable that accepts a TagNode& for configuration + * @throws std::invalid_argument if the tag name is invalid or if the + * consumer throws an exception + * + * Example usage: + * @code + * container.append("error", [](TagNode& error) { + * error.append("message", [](TagNode& msg) { + * msg.append("Something went wrong"); + * }); + * }); + * @endcode + */ + template + void append(const std::string &tagname, Consumer &&consumer) { + TagNode *tagNode = new TagNode(tagname); + try { + consumer(*tagNode); + this->NodeContainer::append(tagNode); + } catch (...) { + delete tagNode; + throw; + } + } + /** * Serializes the node into an XML formatted string. */ @@ -78,8 +115,8 @@ class TagNode : public ITagname, public IAttributes, public NodeContainer { /** * Stream operator overload for easy serialization */ -inline TagNode &operator<<(TagNode &node, std::shared_ptr child) { - node.append(std::move(child)); +inline TagNode &operator<<(TagNode &node, INode *ptrNode) { + node.NodeContainer::append(std::move(ptrNode)); return node; } @@ -88,8 +125,13 @@ inline TagNode &operator<<(TagNode &node, std::shared_ptr child) { * child containing the provided text. */ inline TagNode &operator<<(TagNode &node, const std::string &withText) { - auto textNode = std::make_shared(withText); - node.append(textNode); + TextNode *_ptrNode = new TextNode(withText); + try { + node.NodeContainer::append(_ptrNode); + } catch (...) { + delete _ptrNode; + throw; + } return node; } } // namespace xtrpg::xml::node \ No newline at end of file