Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add void* user data to xml_node_struct #76

Closed
baixuefeng opened this issue Jan 5, 2016 · 12 comments
Closed

Add void* user data to xml_node_struct #76

baixuefeng opened this issue Jan 5, 2016 · 12 comments

Comments

@baixuefeng
Copy link

like this:

    struct xml_node_struct
    {
            ...
            xml_attribute_struct* first_attribute;
            void* user_date;
    };

and add 2 function mebers to xml_node like this

PUGI__FN void xml_node::set_user_data(void* p)
{
    if (_root)
    {
         _root->user_date = p;
    }
}

PUGI__FN void* xml_node::get_user_data()
{
    return _root ? _root->user_date : 0;
}

english is pool , be sorry : (

@DasNaCl
Copy link

DasNaCl commented Jan 5, 2016

Why would you need user data on a pugixml node!?


Please provide a minimal code example!

@baixuefeng
Copy link
Author

Because I want associate it with a handle_less window : )
If so, I don't need to reimplement a tree structure.

@baixuefeng
Copy link
Author

In short, I wish it is not only a xml parser, but also a tree manager !

@zeux zeux changed the title please add a data maber(void* user_data) to xml_node_struct Add void* user data to xml_node_struct Jan 10, 2016
@zeux
Copy link
Owner

zeux commented Jan 10, 2016

Note that you can use separate lookup tables (e.g. std::map<xml_node, T> or std::unordered_map<xml_node_struct*, T> - you can get the pointer to xml_node_struct using internal_object() method) to solve this without changing pugixml.

The problem with adding this is memory cost. This increases the DOM size by ~12% in the default mode; in compact mode this would either increase the node size ~2x for some platforms (e.g. on x64 it'd go from 12 bytes to 24 because of alignment), or would be implemented as a separate lookup table so you might as well do it yourself externally.

@baixuefeng
Copy link
Author

With "map" solution, compared to the increase of a user data "void* ", the overall memory consumption, time consumption will be greater. Of course, when you don't use the "user data", the memory is wasted.
What about adding a macro switch, Similar to PUGIXML_WCHAR_MODE?

@igagis
Copy link
Contributor

igagis commented Feb 14, 2016

In my opinion, as long as this feature is needed by only small number of people (only 1 so far) you have to go with std::map solution. Because otherwise, huge amount of people who does not need this feature (all other people so far) will waste 12% of memory just for the sake of one man convenience.
Propose to freeze this issue until it gets significant number of votes.

@ybungalobill
Copy link

Notice that you could as well add a configuration to xml_document to set how much extra bytes to allocate per node, which if by default is set to zero then will not cost anything for those who don't use this feature. Yet I'm not convinced this a useful feature. User data pointers are used mostly for associating callbacks with concrete user instances, but here we don't have callbacks originating from pugixml, so its usage is somewhat moot.

@zeux
Copy link
Owner

zeux commented Jul 16, 2016

Additionally, my experience has been that when you do want to attach metadata to xml_nodes, you generally only want a partial subset to have one, or you want different subsets to have different metadata - I usually use map or unordered_map for this and it works reasonably well (of course it's space inefficient, which could be solved with a better hash map implementation).

Anyway, I'm closing this for now since there does not seem to be an awful lot of demand. If it does reappear my inclination is to add an extra define to keep the code simpler (at the expense of an extra amount of configurations - which in this case would be probably left untested).

@zeux zeux closed this as completed Jul 16, 2016
@zeux zeux added the wontfix label Jul 16, 2016
@falemagn
Copy link

Incidentally, just found the need for a "user data" field of sort in the xml node. In my case, a simple boolean would be enough, as I just need to flag the node. I could do it by adding an attribute to it, or keeping a separate unordered_map, but those would waste more memory and CPU than having a boolean flag embedded in the node itself.

@cxw42
Copy link

cxw42 commented Sep 19, 2021

In short, I wish it is not only a xml parser, but also a tree manager !

One more vote for this feature :) . I've been looking, and there doesn't seem to be consensus around a single generic tree-with-named-nodes library supporting C++11. Adding a userdata option would permit pugixml to fill this niche.

Thanks for considering this request!

@igagis
Copy link
Contributor

igagis commented Sep 19, 2021

@cxw42 check this generic tree implementation:
https://github.com/cppfw/utki/blob/master/src/utki/tree.hpp

@cxw42
Copy link

cxw42 commented Sep 19, 2021

@igagis Nice --- thanks very much for the pointer! I hadn't seen that project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants