varunsrin / rusty_money

Money library for Rust
MIT License
86 stars 32 forks source link

Add Serde Serialize #27

Closed Sneagan closed 3 years ago

Sneagan commented 4 years ago

This adds serde serialize to Money. I would have liked to get it working for Deserialize as well, but the 'static type of the Currency field posed a bit of a challenge.

Example:

{
  "amount":"16.27",
    "currency":{
      "locale":"EnUs",
      "exponent":2,
      "iso_alpha_code":"USD",
      "iso_numeric_code":"840",
      "name":"United States Dollar",
      "symbol":"$",
      "symbol_first":true,
      "minor_denomination":1
    }
}
Sneagan commented 4 years ago

Looks like there were some conflicts. I hope I resolved them to your satisfaction. Please let me know if I need to change anything. :)

varunsrin commented 4 years ago

First off, thanks for taking the time to write this!

What use case does the serialization alone cover for you? I wasn't planning on adding this until I got both working, but open to suggestions on why this might be useful.

Sneagan commented 4 years ago

I am generating Money objects from external data and then serializing it for transmission to another system. Deserialize would be great as well, but as you note that is more complicated and hot something I need myself immediately.

varunsrin commented 4 years ago

Ok, thanks for the explanation.

I'm going to think through the design for this a bit more before approving. For instance, if I was trying to store a money representation in a DB, I wouldn't want to serialize the whole struct, perhaps just the currency code and amount, and reconstruct the object from the library as needed. That requires json parsing with this implementation, which I'd like to avoid.

Sneagan commented 4 years ago

Fair enough! Format is pretty specific to the use case. For what you're describing I might advocate a method to generate a formatted string from the value. I tend to think of serialization as a mostly lossless conversion for the purpose of sharing data between services rather than as a mechanism to generate DB-friendly content. However, with sufficiently sophisticated deserialization it could work just fine with a string representation.

seanpianka commented 4 years ago

@varunsrin Hello and thanks for creating this crate! Has there been any progress on the efforts to add support for deserialization with serde?

sjoerdsimons commented 4 years ago

fwiw one big issue with just deriving serialize on the money struct is that you're exposing the internal struct fields in your serialization, which would break as soon as the fields change. The serialization should really be seperate from the libraries data layout.

fwiw for the personal for which i'm using this crate I just serialize the currency and amount into one string; and deserialize them via the money::from_string method.

varunsrin commented 4 years ago

i agree with separating serialization logic from the underlying struct. there's also other stuff i want to think through like what would make this easy to put into a SQL database for example. maybe even including a postgres adapter that you could import if needed.

i'm currently working on allowing generic money types (there's a PR out that needs some feedbac). that will change the internal structure a bit, and then im going to sit down and think through serialization after. no timeline yet unfortunately @seanpianka

varunsrin commented 3 years ago

closing this PR, since the library has changed quite a bit and the serde behavior needs to be rethought. adding it to the roadmap.