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

Lazy loading support. #17

Closed
wants to merge 13 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@drmohundro
Copy link
Owner

drmohundro commented Feb 24, 2015

At this point, it is a very early work in progress. I'm hesitant to even publish the code given the TODOs and debug print statements, but it is at least a start.

Known issues:

  • Lack of tests - I only have one test related to lazy loading
  • Integer indexing hasn't been started, only string indexing
    • This means that only simple xml like <root><element>Foo</element></root> will work with lazy loading for now
  • Refactoring and rethinking of the public API

I'd welcome feedback on whether or not the public naming makes sense. At the moment, you call SWXMLHash.lazy to perform lazy loading. I thought it might make sense to make lazy loading the default (and it still may), but basic error handling will change because we won't be able to begin parsing the XML until element is called. This means that code that checks for .Error directly off of an indexing operation won't work. It's at least something to think about.

This is the work to handle issue #11.

@drmohundro drmohundro force-pushed the lazy-loading branch from e3c6268 to 0440c3c Feb 24, 2015

@drmohundro drmohundro force-pushed the lazy-loading branch from 0440c3c to 95e2886 Feb 26, 2015

@drmohundro

This comment has been minimized.

Copy link
Owner

drmohundro commented Feb 28, 2015

As a temporary measure to ensure I keep as much functionality as possible, I've copied the tests from standard parsing and have modified the copy to do lazy parsing. Currently, there are 13 tests that are failing (with the majority of them revolving around index parsing).

I think I'll ultimately end up splitting XMLParser into two classes - one for standard parsing and one for lazy parsing. I'd rather follow SRP a little more closely. First, though, I need to get the tests passing.

@drmohundro

This comment has been minimized.

Copy link
Owner

drmohundro commented Mar 3, 2015

Integer indexing now should work in at least most cases.

What still remains (that I know of):

  • Clean the code up
  • all and children don't work
  • withAttr doesn't work
  • Multiple calls don't seem to work (it lazy loads only once right now...)
  • Decide what error handling will look like

drmohundro added some commits Mar 3, 2015

First steps towards lazy loading support.
Some notes:
- This is NOT done. The code needs to be refactored a LOT before this is
  considered done.
- Simple string indexing should work, but int indexing isn't implemented
  yet.
- More test cases need to be added.
Get lazy element parsing working.
Index parsing still doesn’t work, but I have some ideas.
Get indexing working with lazy loading.
This code only pulls in elements that match the "path" of the indexing. In other words, if asking for xml["elem"]["book"][1]["author"], we should *only* return elements that match 1) the names, 2) the hierarchy, and 3) the count. With indexing, we have to index enough elements until we reach the number requested.

I've introduced code to try to short circuit the checks for performance reasons - the checks spin all matched elements in backwards order and fail out as soon as a mismatch is found. One caveat, if there isn't a match, this will spin the entire XML document looking for the requested elements.

@drmohundro drmohundro force-pushed the lazy-loading branch from 6336cb1 to a356508 Mar 3, 2015

drmohundro added some commits Mar 24, 2015

Refactor and simplify lazy loading.
Introduced a separate lazy loaded XML parser instead of using branching
logic in the existing parser - this drastically simplifies the code.
Also started using built-in `startsWith` instead of my hand-coded one.
@drmohundro

This comment has been minimized.

Copy link
Owner

drmohundro commented Mar 25, 2015

All tests now pass except 2 that are related to errors. More specifically, in the non-lazy scenario, non-matching searches return an NSError object; however, with lazy parsing, it currently returns nil. I think this should be acceptable for the lazy loaded approach, but I'm open to objections!

@drmohundro

This comment has been minimized.

Copy link
Owner

drmohundro commented Mar 25, 2015

I opted to merge this into the xcode-6.3 branch instead. It will go to master when the xcode-6.3 branch does.

@drmohundro drmohundro closed this Mar 25, 2015

@drmohundro drmohundro deleted the lazy-loading branch Mar 25, 2015

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