{"action":"created","issue":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/34","repository_url":"https://api.github.com/repos/MangelMaxime/Thoth","labels_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/34/labels{/name}","comments_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/34/comments","events_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/34/events","html_url":"https://github.com/MangelMaxime/Thoth/pull/34","id":332191680,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0NzM2MTI3","number":34,"title":"add object decoder","user":{"login":"glennsl","id":5207036,"node_id":"MDQ6VXNlcjUyMDcwMzY=","avatar_url":"https://avatars3.githubusercontent.com/u/5207036?v=4","gravatar_id":"","url":"https://api.github.com/users/glennsl","html_url":"https://github.com/glennsl","followers_url":"https://api.github.com/users/glennsl/followers","following_url":"https://api.github.com/users/glennsl/following{/other_user}","gists_url":"https://api.github.com/users/glennsl/gists{/gist_id}","starred_url":"https://api.github.com/users/glennsl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/glennsl/subscriptions","organizations_url":"https://api.github.com/users/glennsl/orgs","repos_url":"https://api.github.com/users/glennsl/repos","events_url":"https://api.github.com/users/glennsl/events{/privacy}","received_events_url":"https://api.github.com/users/glennsl/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-06-13T22:46:20Z","updated_at":"2018-06-14T06:05:27Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/pulls/34","html_url":"https://github.com/MangelMaxime/Thoth/pull/34","diff_url":"https://github.com/MangelMaxime/Thoth/pull/34.diff","patch_url":"https://github.com/MangelMaxime/Thoth/pull/34.patch"},"body":"This should be considered more of a proposal than a complete and mergable implementation, mostly because the ergonomics isn't as good as I hope it can be, but it could also use some refactoring and clean-up. As it is, I've mostly tried to emulate the behavior of the existing `field` and `at` decoders, which is a bit awkward with this approach.\r\n\r\nThere are several reasons for this proposal:\r\n* Using `map` and the \"pipeline\" to decode objects is error-prone as it relies on disassociated positional arguments. So if you order the arguments wrong, you lose.\r\n* `field and `at` mixes several concerns. It does not properly separate the validation of the object with that of the field. Even if you can pattern match on the different kinds of errors, combinators like `option` don't do that, and it's not clear how you'd tweak that API to do so without becoming very verbose.\r\n\r\nThe only major downside I've found is that you can't use the existing decoder combinators, such as `optional` and `either`. You can use `field.optional` with `Option.orElse`, for example, though unfortunately that doesn't seem to exist in F# yet.\r\n\r\nHere's an example of how it currently looks in use:\r\n\r\n```fsharp\r\nobject <| fun { field = field; at = at ] -> {\r\n  name = field.required \"name\" string;\r\n  posX = at.required [\"pos\"; \"x\"] float;'\r\n  posY = at.required [\"pos\"; \"x\"] float;\r\n  height = field.optional \"height\" int\r\n}\r\n```\r\n\r\nAlso note that this is an idea that originates from a bit of experimentation I'm doing on [my Reason/BuckleScript JSON library](https://github.com/glennsl/bs-json). And if some good ideas pop up here, I'll most likely steal it ;)"},"comment":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/comments/397181475","html_url":"https://github.com/MangelMaxime/Thoth/pull/34#issuecomment-397181475","issue_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/34","id":397181475,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NzE4MTQ3NQ==","user":{"login":"MangelMaxime","id":4760796,"node_id":"MDQ6VXNlcjQ3NjA3OTY=","avatar_url":"https://avatars0.githubusercontent.com/u/4760796?v=4","gravatar_id":"","url":"https://api.github.com/users/MangelMaxime","html_url":"https://github.com/MangelMaxime","followers_url":"https://api.github.com/users/MangelMaxime/followers","following_url":"https://api.github.com/users/MangelMaxime/following{/other_user}","gists_url":"https://api.github.com/users/MangelMaxime/gists{/gist_id}","starred_url":"https://api.github.com/users/MangelMaxime/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MangelMaxime/subscriptions","organizations_url":"https://api.github.com/users/MangelMaxime/orgs","repos_url":"https://api.github.com/users/MangelMaxime/repos","events_url":"https://api.github.com/users/MangelMaxime/events{/privacy}","received_events_url":"https://api.github.com/users/MangelMaxime/received_events","type":"User","site_admin":false},"created_at":"2018-06-14T06:05:27Z","updated_at":"2018-06-14T06:05:27Z","author_association":"OWNER","body":"> Using map and the \"pipeline\" to decode objects is error-prone as it relies on disassociated positional arguments. So if you order the arguments wrong, you lose.\r\n\r\nIndeed, we have some problem with the arguments ordering. And didn't find yet a solution to resolve it\r\n\r\n> field andatmixes several concerns. It does not properly separate the validation of the object with that of the field. Even if you can pattern match on the different kinds of errors, combinators likeoption` don't do that, and it's not clear how you'd tweak that API to do so without becoming very verbose.\r\n\r\nI am not sure to understand this point. Would you be able to explain show me code with current API of Thoth.Json that is failing ?\r\n\r\n-----\r\n\r\nThis is kind of unfortunate that you can't use existing decoders. And I have to admit I am not a big fan of the current signature of this new API but we can think about it when I understand the problem you are trying to archieve :).\r\n\r\nAnother question, could you please explain me what is this `builder` thing ? https://github.com/MangelMaxime/Thoth/pull/34/files#diff-fcf19738bdd635ced3cec659bd7097a9R229"}}
{"actor":{"display_login":"MangelMaxime"}}