search

danthorpe / Money

Swift value types for working with money & currency
MIT License
933 stars 91 forks source link

Refactor FX functionality into danthorpe/FX repository. #36

Closed irace closed 8 years ago

irace commented 8 years ago

Perhaps not a huge deal but it does seem a bit weird for a money formatting library to depend on a JSON parsing library. Is this a necessary dependency? I presume this will mean that most apps that include Money end up including two JSON libraries in their applications, unintentionally.

danthorpe commented 8 years ago

Yeah, I totally hear you on this.

It's used by the remote FX services stuff. e.g. Yahoo currency exchange. These are the options:

  1. Write my own bespoke JSON parser
  2. Don't include the FX services stuff
  3. Include it and see how much of an impact it makes, do people complain?
  4. Wait for better cross dependency support with SPM.

Essentially, my goals with Money were to write it in a week. It ended up taking two weeks, including the FX stuff. I was pragmatic and opted to use a dependency for the JSON parsing. Eventually I'm kinda thinking that this sort of thing will be worked out - I think I'm correct in thinking that cross dependency resolution will be part of SPM. So, in the grand scheme of things, especially regarding Swift, I was more focused on trying to write a clean API and not worry too much about how the sausage is made. Everything is subject to change.

However, if people are using Money especially on  TV or  Watch, I'd be happy to look into getting rid of the JSON & Result dependencies. One possible minimal effort option is to move the FX stuff into an extra module.

What do you think?

irace commented 8 years ago

One possible minimal effort option is to move the FX stuff into an extra module.

This is the obvious solution in my opinion. Money could be a lightweight formatting library with minimal dependencies. FX could be a library with networking capabilities that depends on Money.

mrdavey commented 8 years ago

I agree that FX could/should be moved into an extra module.

danthorpe commented 8 years ago

Okay, that's what I'll do.

danthorpe commented 8 years ago

Getting https://github.com/danthorpe/FX/issues/1 setup. But, first, it'll make sense to remove the dependencies & code from here. Especially SwiftJSON which is proving problematic for watchOS.

danthorpe commented 8 years ago

This has now been done as of version 1.5.1 of Money.