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

Allow decoding Decimal from regular numbers #26

Open
Cyberbeni opened this issue Mar 19, 2020 · 10 comments
Open

Allow decoding Decimal from regular numbers #26

Cyberbeni opened this issue Mar 19, 2020 · 10 comments

Comments

@Cyberbeni
Copy link

Decimals are originally meant to be encoded/decoded as a dictionary: https://github.com/apple/swift/blob/3978d81c840c899bd090243d7d344f8dceacd74b/stdlib/public/Darwin/Foundation/Decimal.swift#L118
Apple's JSONDecoder provides a way to decode them from single numbers but sadly numbers are parsed as Double by JSONSerialization, so it loses precision: https://github.com/apple/swift/blob/3978d81c840c899bd090243d7d344f8dceacd74b/stdlib/public/Darwin/Foundation/JSONEncoder.swift#L2464

Since pure-swift-json parses numbers as string, we could provide a similar way of parsing Decimal from regular numbers. As far as I can understand, JSONSingleValueDecodingContainter would need a special decode function for Decimal.Type that would work similarly to the second link.

Decimal's init also takes a Locale, it should be allowed to be customized and the default should probably be "en_US".

Most likely a separate issue but there should also be an option for encoding Decimals the same way instead of using the default method.

@Cyberbeni
Copy link
Author

Related Swift bugreport: https://bugs.swift.org/browse/SR-7054

@fabianfett
Copy link
Collaborator

Hi @Cyberbeni,

I get your point. Thanks for explaining here. Of course I could add a special decode function to JSONSingleValueDecodingContainter. The only problem is: You won't be able to access this function.

When you implement your own init(decoder:) and ask for a singleValueContainer() you will get a SingleValueDecodingContainer. You will not be able to cast this protocol to JSONSingleValueDecodingContainer because this is an implementation detail that should stay private and will not be exposed.

That's why I'm afraid the real solution to your problem is to propose a change to the Swift STL which would add a function decodeNumberAsString() to the SingleValueDecodingContainer protocol.

Does that make sense?

@Cyberbeni
Copy link
Author

I realized that you have to do it in the generic decode function and for all 3 types of containers.

@fabianfett
Copy link
Collaborator

It‘s not so much a problem of my containers but the container protocol.

@Cyberbeni
Copy link
Author

func decode<T>(_ type: T.Type) throws -> T where T : Decodable {
    if type.self == Decimal.Type,
         case let .number(number) = value {
        // Decode Decimal from String
    } else {
        // Decode normally
    }
}

And similarly for the other 2 containers.

@fabianfett
Copy link
Collaborator

fabianfett commented Mar 20, 2020

Okay. That would work, but in this case I would need to link Foundation - which kinda is not the goal of this Endeavour. But you‘re free to fork any time... @Cyberbeni Does this work for you?

@xanderdunn
Copy link

I also came here in hopes of finding a solution to the JSON->Decimal problem. This blog post is a good description of the issue in Swift.

@Cyberbeni
Copy link
Author

(I don't need this at my current job, that I also had when I opened this issue, the last 2 comments from March should lead you to a relatively easy solution @xanderdunn )

@xanderdunn
Copy link

Thanks @Cyberbeni, your solution was to fork the codebase and modify this func decode<T> to include a special case for Decimal?

@Cyberbeni
Copy link
Author

yes

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

No branches or pull requests

3 participants