Various additions and refactorings

[frasertweedale]

I went crazy with the refactoring mallet.

And by crazy I mean sane. I did nice things to your library George. With my mallet.

Most changes are uncontroversial. The debatable changes include:

• removing access to the Sum a. I have not found it useful so I removed it, but as suggest in the commit msgs if it is actually useful, I'd personally prefer to see a Wrapped instance.

• Rename getMoney (which was previously a Lens) to money (it is now an Iso).

There was previously a comment that "dividing money by money is nonsense". It is actually quite useful, hence ($/$), and I changed the comment s/dividing/multiplying/ which is... not as overtly false? (related reading: https://math.stackexchange.com/questions/1236692/how-would-multiplying-money-work/1236772#1236772)

[gwils]

Thanks Frase!

Removing optic access to the Sum is fine. It was Sum underneath so that we could say "deriving" to get the additive monoid. Now I think it would be easier to remove the use of Sum and define + and 0 ourselves rather than newtyping a newtype. This would let us drop GeneralizedNewtypeDeriving too if we wanted. I will make that change myself after this PR is merged.

I like the explicit exports but I don't like that the constructor is not exported. The Iso provides mostly the same functionality, sure, but exporting the constructor itself enables type safe zero-cost coercion: https://wiki.haskell.org/GHC/Coercible Can you please add the constructor to the exports?

Based on your stack exchange article justifying $/$, I'm thinking we should also define $*$. I think its return type would be Money (Money a). To me it seems odd to have divide but not multiply. What do you reckon?

[frasertweedale]

On Mon, Jul 10, 2017 at 06:21:06PM -0700, George Wilson wrote:

Thanks Frase!

Removing optic access to the Sum is fine. It was Sum underneath so that we could say "deriving" to get the additive monoid. Now I think it would be easier to remove the use of Sum and define + and 0 ourselves rather than newtyping a newtype. This would let us drop GeneralizedNewtypeDeriving too if we wanted. I will make that change myself after this PR is merged.

I like the explicit exports but I don't like that the constructor is not exported. The Iso provides mostly the same functionality, sure, but exporting the constructor itself enables type safe zero-cost coercion: https://wiki.haskell.org/GHC/Coercible Can you please add the constructor to the exports?

Based on your stack exchange article justifying $/$, I'm thinking we should also define $*$. I think its return type would be Money (Money a). To me it seems odd to have divide but not multiply. What do you reckon?

Actually, ($/$) is something I am already using in my tax lib. The article was more for discussion around whether multiplication makes sense.

The implications of Money (Money a) need to be thought through. We can define:

($*$) :: Num a => Money a -> Money a -> Money (Money a)
($*$) m n = review money (over money (* view money n) m)

Which works as expected. But can we use ($/) to get back to plain old money^1?

λ> :t (m1 $*$ m2) $/ m1
(m1 $*$ m2) $/ m1 :: (Fractional (Money a), Num a) => Money (Money a)

We don't have an instance Fractional (Money a) and I don't think we want one.

The alternative is to define a dedicated function for "powering down" your money:

λ> let mm $$/$ m = view money mm $/ view money m
λ> :t ($$/$)
($$/$) :: Fractional a => Money (Money a) -> Money a -> Money a

Which at least does the right thing:

λ> let mm $$/$ m = view money mm $/ view money m
λ> :t ($$/$)
($$/$) :: Fractional a => Money (Money a) -> Money a -> Money a
λ> let ($*$) m n = review money (over money (* view money n) m)
λ> let m1 = review money 2
λ> let m2 = review money 3
λ> let mm = m1 $*$ m2
λ> mm
$$6
λ> mm $/ m1

<interactive>:12:1:
No instance for (Fractional (Money a0)) arising from a use of ‘it’
In the first argument of ‘print’, namely ‘it’
In a stmt of an interactive GHCi command: print it
λ> mm $$/$ m1
$3.0
λ> mm $$/$ m2
$2.0

But the lack of Num and Fractional instances for Money (Money a) still makes it quite hard to work with higher-power money. We'd have to define lots more combinators.

Over to you.

[gwils]

Hmm, let's kick $*$ down the road for later thought :)