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

Parsing objects no longer works #54

Open
benjaminsnorris opened this issue Jul 28, 2016 · 10 comments · Fixed by #91
Open

Parsing objects no longer works #54

benjaminsnorris opened this issue Jul 28, 2016 · 10 comments · Fixed by #91

Comments

@benjaminsnorris
Copy link
Contributor

We recently updated to v0.9.6, and found that some parsing is now broken. Previously, we could do

let dictionary: [String: Bool] = try object <| key

Now, I have to do something more like:

guard let dictionary = try object.anyForKey(key) as? [String: Bool] else { throw _ }

Another example is that I previously did this without problem:

let dictionary: [String: [String: JSONObject]] = try object <| key
@jarsen jarsen added the bug label Oct 27, 2016
@KingOfBrian
Copy link
Contributor

Hey @benjaminsnorris -- can you try out #86 ? I added a quick test for it and it was failing on master with value(for:. That's now working, and I'm guessing that the <| will work, but I'm not sure.

@benjaminsnorris
Copy link
Contributor Author

Thanks @KingOfBrian for checking on this. It seems to still be broken. Here is an example from a quick playground I made:

//: Playground - noun: a place where people can play

import UIKit
import Marshal

let teamJSON: JSONObject = [
    "name": "Engineering",
    "managerId": "manager.id",
    "teamMemberIds": [
        "teamMember.id": true,
        "secondTeamMember.id": true,
        "thirdTeamMember.id": true,
    ],
]

struct Team: Unmarshaling {
    var name: String?
    var teamMemberIds = [String: Bool]()
    var managerId: String?

    init(object: MarshaledObject) throws {
        if let teamMemberIdDictionaries: [String: Bool] = try object.value(for: "teamMemberIds") {
            teamMemberIds = teamMemberIdDictionaries
        } else {
            teamMemberIds = [:]
        }
        managerId = try object <| "managerId"
        name = try object <| "name"
    }

}

let team = try? Team(object: teamJSON)
team?.teamMemberIds

@KingOfBrian
Copy link
Contributor

Thanks for the snipit, that actually pointed out a bug in the non-operator code. PR coming shortly.

@benjaminsnorris
Copy link
Contributor Author

Just in case it's the same thing, with the latest changes, parsing [String: JSONObject] also broke. If it's unrelated, I'll create a separate issue.

@KingOfBrian
Copy link
Contributor

I believe that is a known issue about not supporting nested containers (#76). I believe we can fix it, but the code duplication gets painful. I'd log an issue, note #76 and see what he thinks.

@KingOfBrian
Copy link
Contributor

Can you try out #91 @benjaminsnorris ? I think that will fix the operators, and you found a fun bug in the optional dictionary extraction which should force a new version unfortunately.

Also, just a note from your snip-it, I really like the succinctness of the ?? for optional value for's that default to an empty container.

teamMemberIds = try object.value(for: "teamMemberIds") ?? [:]

@benjaminsnorris
Copy link
Contributor Author

@KingOfBrian sweet! If we get this working, I would love to be able to use the succinct version of the optional parsing. That will help clean things up considerably.

@benjaminsnorris
Copy link
Contributor Author

@KingOfBrian unfortunately, it does not work. It parses just fine when the key is there, but when the key is not there, I get the Key not found error.

@KingOfBrian
Copy link
Contributor

Great catch @benjaminsnorris -- That was actually a bug, if you update that other branch it should work. I blame the cold.

@jarsen jarsen reopened this Dec 15, 2016
@jarsen
Copy link
Member

jarsen commented Dec 15, 2016

whoops not sure how this got closed by that PR

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

Successfully merging a pull request may close this issue.

3 participants