I'm building a clone banking app at the moment, and one of the things I'm trying to do is add Split Transaction (which can then be shared with a set of friends paying a given amount each).

Initially, the transaction is split equally amongst friends (unless it doesn't split equally, in which case the remainder gets added on to one unlucky friend). The user can then manually adjust the amount each pays, which then updates the others. If the user has manually adjusted an amount for a friend, this friends split doesn't get updated automatically when the user adjusts another friend's amount (i.e. if the user says friend1 pays £12, it will always be £12 until the user says otherwise).

I've been fiddling for a while trying to make the method as concise and Swifty as possible - but I'd really appreciate any feedback on my approach.

For the purposes here, I'm only trying split the money equally between people (but I still wanted to explain the user-defined split so that the current code makes sense).

I'm using https://github.com/Flight-School/Money to represent the transaction value, all within a Transaction class. I need to round quite a bit to ensure the split and remainder stick to 2 decimal places. Here's the relevant code:

A struct to hold an amount along with if the user set it or not (needs to be a custom object for codable reasons):

I think the problem I'm finding is the code makes perfect sense to me, but I'm not sure how clear it is to an outside reader. Any thoughts on my approach would be much appreciated (and sorry for the long question!). And I'd also love to know if there's any way to write this more concisely!

2 Answers
2

Separation of concerns

There is (in my opinion) too much logic in the main function. Splitting an amount “fairly” into \$ n \$ parts is difficult enough. That function should not know about “friends,” that a list of friends might not be unique, or that friends are represented as strings.

I would suggest to define a function

splitAmount(total: numParts: )

which takes an amount and a number and returns an array of amounts (the exact signature is discussed later). Your splitTransaction() function can then build on splitAmount().

Why only GBP? – Make it generic!

Your splitTransaction() function takes a Money<GBP> amount, i.e. it works only with pound sterling, the British currency. Only minimal changes are required to make the same function work with arbitrary currencies: Change the function type to

The algorithm

In the first example, the remainder is \$0.02\$, and a better result would be achieved by splitting that among two friends:

10.04 -> 1.68, 1.68, 1.67, 1.67, 1.67, 1.67

In the second example, the remainder is \$-0.03\$. Again the split is not optimal, in addition the first amount is less then the others, contrary to the description of your method. A better result would be

10.05 -> 1.68, 1.68, 1.68, 1.67, 1.67, 1.67

The problem is of course the floating point arithmetic. Even if Money uses the Decimal type which can represent decimal fractions exactly: It still cannot represent the result of a division like \$ 10.05 / 6 \$ exactly. Rounding that fraction to a multiple of \$ 0.01 \$ (the “minor unit” of the currency) can result in a smaller or a larger value (as in the second example).

Actually such calculations are much better done in integer arithmetic. If £10.04 are represented as 1004 (pennies), then we can perform an integer division with remainder

1004 = 167 * 6 + 2

so that each part is 167 pennies, plus 1 penny for the first two parts. Similarly:

1005 = 167 * 6 + 3

Unfortunately, the Money class does not provide methods to get an amount as integer multiple of the currency's minor unit, but that is not too difficult to implement:

\$\begingroup\$I've had couple of great answers for this - thank you! I've selected this answer as it's the clearest (and most understandable at my level - although that's totally subjective!).\$\endgroup\$
– ADBMay 29 at 11:06

\$\begingroup\$Out of interest, how would you extend this for negative amounts please? I've tried just using the absolute value for units but I'm not sure if it's as simple as that.\$\endgroup\$
– ADBJun 3 at 11:25

1

\$\begingroup\$@ADB: I have added a possible implementation which works for both positive and negative amounts.\$\endgroup\$
– Martin RJun 3 at 20:47

\$\begingroup\$That's so helpful - thank you so much.\$\endgroup\$
– ADBJun 4 at 7:21

When splitTransaction is diving into Decimal types and the like, that indicates that you’ve placed algorithms at the wrong level.

To that end, let’s give the Money the necessary arithmetic operators so that splitTransaction doesn’t get into the weeds of Decimal or other details of the Money type.

You have a rounded computed property. I’d suggest making that a function and allowing the caller to specify the type of rounding to be applied.

You have made Currency conform to Codable (which you presumably did so you could make Money also Codable). I don’t think you intended to do that because currencies will have all sorts of properties that you most likely don’t want to encode. That should not be codable, and Money should have the currency code as a property, not the currency itself. That code is presumably the only thing you want Money to encode.

Your algorithm assumes there will be one user who picks up the remainder. I’d suggest you spread it around. If the bill is €9.02, rather than sticking the first person with €3.02, let’s give two people €3.01 and the third €3.00.

You are applying the remainder to the first user. I’d suggest you randomize it. If the list was sorted, for example, you don’t want the same people always getting the short end of the stick.

Your rounding algorithm presumably assumes that you’ll be rounding to the nearest pence. But you should handle currencies that generally don’t use fractional values. In fact, going a step further, you should really handle arbitrary minimum units for the currency. E.g. imagine a world where the US decides it doesn’t want to deal with pennies any more, and makes the nickel the minimum unit. Well, that’s how you should be rounding (to the nearest $0.05), not to two decimal places. Likewise, imagine that Japan decided that they weren’t going to deal with anything smaller than ¥20. Then, again, that’s what you should be rounding to, not to the nearest yen.