Dividing an arbitrary dollar amount into the fewest bills and coins

I am taking a class in mobile application development this fall. I have finished my first assignment of creating a class to divide an arbitrary dollar amount into the minimum number of bills and coins. I have completed the assignment and everything works as intended (outside of an occasional error I'm having with computing pennies, but I am working on that). My main reason for posting is I would like to try and make my code more Swift like. I am coming from a mostly C background so I am learning the ins and outs of Swift and OOP on the fly. So if you have any suggestions to making my code more "Swift like" I would greatly appreciate it.

public class Cash
{
//    Denominations of monies
let bills = [50.00, 20.00, 10.00, 5.00, 1.00, 0.25, 0.10, 0.05, 0.01]

//    Backing variable
var value:Double

//    Computed Property of minimum number of bills to satisfy the value sent in
public var minBills:[Int]? {
var tempArray = [Int]()
var tempValue: Double
if value >= 0 {
for index in 0..<bills.count {
tempValue = value / bills[index]                  //Calculating the amount of bills
value -= Double(Int(tempValue)) * bills[index]    //Recalculating the value
tempArray.append(Int(tempValue))                  //Adding number of bills needed to tempArray
}
return tempArray                                      //Returns entire tempArray to minBills
}
else {
return nil
}
}

//Constructor
public init(value: Double) {
self.value = value
}
}


The computed property minBills modifies the value property, which would be unexpected to the caller:

let cash = Cash(value: 50.0)
print(cash.minBills!) // [1, 0, 0, 0, 0, 0, 0, 0, 0]  OK!
print(cash.minBills!) // [0, 0, 0, 0, 0, 0, 0, 0, 0]  What?


Better make value a constant property:

let value: Double


and create a mutable copy only for the computation.

let bills = [50.00, 20.00, 10.00, 5.00, 1.00, 0.25, 0.10, 0.05, 0.01]


can be made a type property

static let bills = [50.00, 20.00, 10.00, 5.00, 1.00, 0.25, 0.10, 0.05, 0.01]


because it is the same for each instance.

But why do you need a class at all? This could be just a (free) function

func minimumBills(for value: Double, withDenominations: [Double]) -> [Int]?


instead. Or, if you plan to add more functionality to the Cash type, consider defining it as a struct unless the reference semantics of a class are explicitly needed.

Another possible design would be to define a CoinSystem type with “factory methods” for known coin systems:

public struct CoinSystem {
let denominations: [Double]

public init(denominations: [Double]) {
self.denominations = denominations
}

public func optimalBills(for value: Double) -> [Int]? {
// ... computation here ...
}

public static var usDollar = CoinSystem(denominations:
[50.00, 20.00, 10.00, 5.00, 1.00, 0.25, 0.10, 0.05, 0.01])
}


This would be used as

let usCoins = CoinSystem.usDollar
let optimal = usCoins.optimalBills(for: 12.34)


and allows adding more coin systems easily.

Now let us consider the “main code,” i.e. the computation of

public var minBills:[Int]?


tempArray and tempValue are bad variable names because they do not indicate what the variable stands for. Also tempValue is only needed inside the scope of the for-loop.

Returning early on error conditions simplifies the code and saves one indentation level:

if value < 0 { return nil }


or

guard value >= 0 else { return nil }


Iterating of the bills array is better done with

for bill in bills {
// ...
}


Summarizing these suggestions, the code could look like this:

public struct CoinSystem {
let denominations: [Double]

public init(denominations: [Double]) {
self.denominations = denominations
}

public func optimalBills(for value: Double) -> [Int]? {
guard value >= 0 else { return nil }

var remainingValue = value
var result = [Int]()
for denomination in denominations {
let count = Int(remainingValue/denomination)
remainingValue -= Double(count) * denomination
result.append(count)
}

return result
}

public static var usDollar = CoinSystem(denominations:
[50.00, 20.00, 10.00, 5.00, 1.00, 0.25, 0.10, 0.05, 0.01])
}


You said

.... outside of an occasional error I'm having with computing pennies ...

That is to be expected, because Double is binary floating point number, and cannot represent all (decimal) fractions exactly. Good resources for this topic are

Possible remedies are

• Work with integers only (representing cents, or whatever the lowest possible denomination is).
• Use the Decimal type to represent the monetary values.

Finally: What you are solving is the Change-making problem, and your approach is the “greedy method:” Always pick the largest denomination which is not greater than the remaining amount. This gives the optimal result for so-called “canonical coin systems,” like the one used in the US, but not necessarily for arbitrary coin systems.

The general problem can be solved with dynamic programming, and you'll find further information in the above referenced Wikipedia page.

• Thank you very much for the response and the resources you provided. I will take a careful look at your code and the information you linked to improve my future work! Aug 29, 2018 at 17:46