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

Suggestion userInfo #167

Closed
ChristianSteffens opened this issue Jan 3, 2018 · 8 comments
Closed

Suggestion userInfo #167

ChristianSteffens opened this issue Jan 3, 2018 · 8 comments

Comments

@ChristianSteffens
Copy link
Contributor

Hi,

we're using your framework extensively and it work's perfectly - but we're missing (only) one thing: Some type of userInfo (var userInfo: [AnyHashable : Any] { get }). So that the user can add some custom information to the XMLIndexer.

This could be very similar (if not identically) to the userInfo in the latest Swift Codable protocol (e.g. see section userInfo).

Is this something for your project's roadmap?

@drmohundro
Copy link
Owner

I haven't worked with Codable much yet, but that seems to make sense... basically it would be a generic object to hold user-specified data that would later be accessible during deserialization?

Would you expect it to live on XMLIndexer instances as well as XMLElement and XMLAttribute (for use during custom deserialization)?

Also, just to clarify, but you're thinking specifically for custom deserialization and not manual indexing, correct?

I'm happy to add this as a future enhancement.

@ChristianSteffens
Copy link
Contributor Author

basically it would be a generic object to hold user-specified data that would later be accessible during deserialization?

Exactly - Apple usually uses a dictionary for that purpose.

Would you expect it to live on XMLIndexer instances as well as XMLElement and XMLAttribute (for use during custom deserialization)?

IMHO adding this object to the XMLIndexer would be best.

Also, just to clarify, but you're thinking specifically for custom deserialization and not manual indexing, correct?

Yes, that's right. During our custom deserialization we need to access some information that isn't included in the actual xml file, this would be the perfect place for the userInfo property.

@drmohundro
Copy link
Owner

👍 got it - I'll see if I can get this feature coded soon. Thanks for the feedback!

@drmohundro
Copy link
Owner

One clarifying question around you would imagine implementation... for Codable, it goes on an instance after you new it up. With SWXMLHash, you're calling parse with an optional call to config.

Would you envision this being a config option? Like this:

let xml = SWXMLHash.config {
              config in
              config.userInfo = [ "myUserInfoKey" : myOptions ]
          }.parse(xmlToParse)

// ... later ...
xml.userInfo  // has the instance of userInfo from above?

I think that makes sense to me given that SWXMLHash is in charge of creating the instance of XMLIndexer, but I wanted to double check first.

@ChristianSteffens
Copy link
Contributor Author

I guess this would be fine - Codable does it the same way: During the Decoders creation you could add userInfo entries - so SWXML config would be appropriate.

But I would try to go a step further and allow the user to change / update the userInfo even after the SWXMLHash instance had been configured. This is something really annoying in Swift's Codable implementation: You're only allowed to set the userInfo during the creation / initialisation of the Decoder.

For Apple's typical 'perfect-world' examples (like JSON parsing) this is fine, but in complex object-graphs (I guess an XML is a perfect example for that) it certainly makes sense to update the userInfo even during the actual decoding.

So in short: Setting the userInfo in SWXMLHash's config is a good start and for most usecase probably enough - but I would recommend to allow changes to the userInfo afterwards (i.e. XMLIndexer should have a getter and setter for userInfo).

@drmohundro
Copy link
Owner

FYI, I just pushed #171 to try to get some userInfo support in place.

One thing I want to note is that it wasn't as straightforward as I expected in being able to change userInfo after the fact. The main reason for this is that you can't have an extension method that is both static and mutating. As a result, the deserialize method can't change the indexer passed to it.

It looks like this could be possible by changing deserialize to take in its XMLIndexer as an inout parameter, but that would be a breaking change. I haven't tried that yet, but it might still be an option in the future.

Let me know if it works for you and I can get this merged and released. Thanks!

@ChristianSteffens
Copy link
Contributor Author

Just try it - work's perfectly - using CodingUserInfoKey is the final touch ;-)

For now it's fine that the userInfo can't be change afterwards (actually we have a (quick'n'dirty) helper method for the same issue with Codables - so we'll use that in the meanwhile.

Maybe you could pin the 'updating of the userInfo' to your roadmap. I understand that a breaking change to your API should happens only if it's absolutely necessary (which IMHO isn't the case right now).

@drmohundro
Copy link
Owner

FYI, I just pushed 4.3.5 out - hope this works for you! Thanks again for the feature suggestion!

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