-
-
Notifications
You must be signed in to change notification settings - Fork 8
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 reader MatchingNode results and a signal to stop reading #66
Conversation
84a64db
to
e865c40
Compare
d41dfe5
to
fab0bf1
Compare
@Sammyjo20 Any remarks about this PR? It's gonna be tagged as 3.0 eventually. |
fab0bf1
to
54bb7ec
Compare
Hey @veewee I really like it, instead of the XML string being returned, having an object that allows you to perform further things sounds great. Did you manage to do that while keeping memory low? My only thought so far is on the BC. You could introduce a separate method on the reader to perform this newer kind of match and keep the old one as the simple one, but in all honesty I quite like the change and it keeps it simple. |
I wonder if this will make some of XML wrangler's logic simpler too, especially around the idea I had of being able to continue reading from an element. |
Yes, memory consumption should be similar to what we had before. It still works the same way internally, but it now returns more detailed information (we already had in the reader's internals). This repo comes with stress tests that ensures that there are no memory leaks in the reader and writer btw.
I rather tag a new major than providing 2 separate methods so that I can provide 1 good way of using the tool. That's what major versions are for IMO : telling people things could be done in a better way.
There are some shortcut functions for transforming into a document or decoding. So that part would become slightly easier. |
Sounds great 👍
I wanted to make it easier to chain reading on from an element. So you could do something like: [$elementOne] = $reader->element('food')->get();
$elementOne->reader('something-else')->get(); I wonder if there's a way to chain |
I think that should already be possible as long as you store the XML string in the It will be faster than rereading the whole XML document. Yet it won't have a positive impact on memory cause the XML was already read in memory and you are performing a second read on top of that xml (so more memory). But again: in comparison with re-reading the whole XML document, it should have a possitive impact. |
That is what I have built as a PoC so far 😀 Cool I might carry on with that one then. As always nice work on this PR - and looking to v3! |
Summary
❗ This is a BC break and should be released in a new major version ❗