Skip to content
This repository has been archived by the owner on Feb 8, 2021. It is now read-only.

Adds a Double decoder. #287

Closed
wants to merge 6 commits into from
Closed

Adds a Double decoder. #287

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 26, 2017

First of all, what a great library! I really appreciate the work you've done and I'm using it in some of my projects.

I would like to suggest that decoding an Integer value to a Double does not return nil. For example, if I have the following model:

struct Example {
    let value: Double
}

extension Example: Decodable {
    init?(json: JSON) {
        guard let value: Double = "value" <~~ json else { return nil }
        self.value = value
    }
}

and if I try to decode this json and produce a Example model:

{
    "value": 0
}

Gloss will return a nil object, as it was unable to decode an Integer value to a Double. I think that this is due to the fact that 0 isn't recognised as a Decimal value.

A solution to this problem is adding a specific decoder for Doubles.

public static func decode(doubleForKey key: String, keyPathDelimiter: String = GlossKeyPathDelimiter) -> (JSON) -> Double? {
    return {
        json in
            
        if let number = json.valueForKeyPath(keyPath: key, withDelimiter: keyPathDelimiter) as? NSNumber {
            return number.doubleValue
        }
            
        return nil
    }
}

This allows for the key value to be decoded even when the underlying value is an Integer. This can be useful for APIs that send back 0 instead of 0.0.

@hkellaway hkellaway self-requested a review April 26, 2017 15:06
Copy link
Owner

@hkellaway hkellaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests need to be added

@hkellaway
Copy link
Owner

hi @NausJessy - not sure if you noticed the code review request. tests need to be added to this PR.

@ghost
Copy link
Author

ghost commented Apr 28, 2017

@hkellaway Sorry, I forgot the tests. I will add them as soon as possible and get back to you.

@hkellaway
Copy link
Owner

if you have time, it would be good to also have the complement - Encoder update for Double. if not, i'll do when i have a chance

@ghost
Copy link
Author

ghost commented Apr 30, 2017

I updated the Encoder as to include the Double complement.

Furthermore, I changed all Double test to use the new decoder and added a case for when an Integer is casted to a Double. I am not very experimented with writing tests, so please feel free to give some feedback about them. If needed, I will change them accordingly.

@adeperio
Copy link

adeperio commented Dec 6, 2017

Hey guys, thanks for the work on this PR. This is tripping me up on an issue as well. Any idea when you think you'll be pulling this through? Thanks... :)

@hkellaway
Copy link
Owner

sorry this one got away from me @NausJessy - just rebased and merged manually into develop; so you'll see it in the 2.0.0 release

@hkellaway hkellaway closed this Dec 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants