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

Finder returns ElementInfo with const Element #30

Open
JaimeIvanCervantes opened this issue Nov 15, 2017 · 3 comments
Open

Finder returns ElementInfo with const Element #30

JaimeIvanCervantes opened this issue Nov 15, 2017 · 3 comments

Comments

@JaimeIvanCervantes
Copy link
Contributor

JaimeIvanCervantes commented Nov 15, 2017

I'm trying to use Finder to get a reference to an Element, and then accept a Visitor from this element, but I'm getting the following error:

candidate function not viable: 'this' argument has type 'const svgdom::Element', but method is not marked const

The problem is that Finder returns ElementInfo with const svgdom::Element& e instead of svgdom::Element& e, so the Element can't be modified.

Here's the code that is causing the error:

auto ref = finder.findById(e.getLocalIdFromIri());
if (!ref) {
     return;
}

class MyVisitor : public Visitor {
public:
   virtual void defaultVisit(Element& e) override {
      // Modify Element here
   }
} myVisitor;

ref->e.accept(myVisitor);

Using Finder to modify Elements seems like a useful pattern to me, otherwise I would have to write my own implementation of Finder with non-constant Element, what do you think?

@igagis igagis self-assigned this Nov 15, 2017
@igagis
Copy link
Member

igagis commented Nov 15, 2017

The Finder class actually does caching, so it does not suppose there will be any modifications to the SVG tree, this is why it works with const references.

It is not hard to implement a visitor which will search element by id and return non-const pointer to the element. No caching is needed in this case, I think.

@igagis igagis removed their assignment Nov 15, 2017
@JaimeIvanCervantes
Copy link
Contributor Author

I do need the cache for efficiency, so I implemented my own Finder where I only changed the returned Element to non-constant and I modified the CacheCreator to inherit a Visitor instead of a ConstVisitor. I could make a PR if you're interested.

@igagis
Copy link
Member

igagis commented Nov 16, 2017

Actually I doubt about caching, because when you modify the SVG tree (add/remove elements) it will invalidate the cache, so I think that caching for non-const finder is actually not a good thing.

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

2 participants