-1

I am writing a basic player wallet management class where Integers are added (credited) or subtracted (debited) to a player's wallet where errors are caught by a simple try-catch logic.

Note: It only is meant to handle Integers.

I am attempting to abstract away the try-catch logic into this Wallet class; it should handle anything with credit, debit actions, capture errors and return errors if any found.

Is there a way to move try-catch stuff into a class and yet returns errors?

Lets go through this example:

My current class is as such:

enum WalletError : Error {
    case mustBePositive
    case notEnoughFunds
}

class Wallet {
    public private(set) var balance: Int = 0

    init(amount: Int = 0) {
        self.balance = amount
    }

    func credit(amount: Int) throws {
        guard amount > 0 else {
            throw WalletError.mustBePositive
        }
        handleCredit(amount: amount)
    }

    func debit(amount: Int) throws {
        guard amount > 0 else {
            throw WalletError.mustBePositive
        }
        guard balance >= amount else {
            throw WalletError.notEnoughFunds
        }
        guard (self.balance - amount >= 0) else {
            throw WalletError.notEnoughFunds
        }
        handleDebit(amount: amount)
    }

    // MARK: (Private)

    private func handleCredit(amount: Int) {
        self.balance += amount
    }

    private func handleDebit(amount: Int) {
        self.balance -= amount
    }
}

Testing the debit actions, I have this XCTest function

func testDebit() {
    let w = Wallet()

    XCTAssertTrue(w.balance == 0)

    // Can't debit negative numbers
    XCTAssertThrowsError(try w.debit(amount: -100)) { error in
        XCTAssertEqual(error as? WalletError, WalletError.mustBePositive)
    }

    // can't debit money you don't have
    XCTAssertThrowsError(try w.debit(amount: 100)) { error in
        XCTAssertEqual(error as? WalletError, WalletError.notEnoughFunds)
    }

    // credit $100
    XCTAssertNoThrow(try w.credit(amount: 100))

    // debit $0 should throw error
    XCTAssertThrowsError(try w.debit(amount: 0)) { error in
        XCTAssertEqual(error as? WalletError, WalletError.mustBePositive)
    }

    // debit > balance should throw error
    XCTAssertThrowsError(try w.debit(amount: 101)) { error in
        XCTAssertEqual(error as? WalletError, WalletError.notEnoughFunds)
    }

    // debit $1 should be successful
    XCTAssertNoThrow(try w.debit(amount: 1))

    // balance should be 100-1 = $99
    XCTAssertTrue(w.balance == 99)
}

The problem I'm having here is:

When I write the application, I need to write the try-catch block every time.

I would like the Wallet class to handle all try-catch stuff, and only report errors

I tried this:

// changes to wallet class
func canCredit(amount: Int) throws -> Bool {
    guard amount > 0 else {
        throw WalletError.mustBePositive
    }
    return true
}

func credit(amount: Int) -> Error? {
    do {
        if ( try canCredit(amount: amount) ) {
            handleCredit(amount: amount)
        }
        return nil
    } catch let error {
        return error
    }
}

But now the XCTest doesn't know about the throwing, and I would need to test the canCredit which now only returns true/false.

Further, the credit attempts to return the error, but I can't use XCTAssertThrowsError()

Is there a way where I can:

  1. The Wallet class should only be responsible for credit, debit and validation

  2. Throw all my try-catch stuff logic into this class so I don't have to repeat the try-catch stuff all the time?

I hope I've clarified the issue.

With thanks

zardon
  • 1,601
  • 4
  • 23
  • 46
  • Leave your methods as they are, in your first code, so testing can still test them. Now just add wrapper methods (or indeed a wrapper class) to serve as the public API, catching the errors so they don’t propagate to the caller. In other words, testing sees something that the real caller does not. – matt Apr 15 '18 at 11:33
  • Not written a wrapper class/method to act as a public API before or how it would relate to the tester/real caller does not, but I am interested in finding out more – zardon Apr 15 '18 at 12:00

2 Answers2

1

Make the function throw and hand over the error

func credit(amount: Int) throws {
    try canCredit(amount: amount)
    handleCredit(amount: amount)  
}
  • If canCredit succeeds the code goes to the next line and the credit is handled.
  • If canCredit fails the received error is handed over and the function is exited.
vadian
  • 274,689
  • 30
  • 353
  • 361
  • I will give it a go – zardon Apr 15 '18 at 12:00
  • A quick test seems to work `let w = Wallet() XCTAssertThrowsError(try w.credit(amount: -100)) { error in XCTAssertEqual(error as? WalletError, WalletError.mustBePositive) } try? w.credit(amount: -100) print(w.description)` – zardon Apr 15 '18 at 12:14
  • But wouldn't I still need to do a try-catch in my main code? `try? w.credit(amount: -100)` – zardon Apr 15 '18 at 12:19
1

Your goal seems to be to take a thrown error and transform it into a returned error. That goal seems wrong. Throwing is a form of flow control. Throwing an error is how you "return an error" to the caller. You shouldn't want, in the first place, the thing you seem to want.

Still, you can certain do it. First, just keep your credit(amount:) and debit(amount:) methods exactly as they already are in your first code example, so that your unit tests can test them. Second, in your "real code", don't call those methods. Instead, call additional "wrapper" methods that you supply, that catch the thrown error and transform it into a returned error.

For example:

func realcredit(amount: Int) -> Error? {
    do {
        try credit(amount:amount)
        return nil
    } catch {
        return error
    }
}
matt
  • 515,959
  • 87
  • 875
  • 1,141
  • Oh I think I understand what you mean, I will try to give it a go. I might be just trying to overcomplicate stuff – zardon Apr 15 '18 at 12:32
  • See my code example. You simply return nil if the `try` succeeds and return the error if the `try` fails. – matt Apr 15 '18 at 12:33
  • But I repeat: this seems a very wrong thing to do, because now you are making the caller check whether was an error by adding a condition testing an Optional for `nil`, whereas the ability to know whether there was an error is _built in_ to the notion of saying `try` in the first place. If the caller doesn't care whether there was an error, the caller can say `try?`. If the caller does care, the caller can catch the error. Saying `try` _is_ a form of condition. – matt Apr 15 '18 at 12:34
  • What I'm trying to do is get the class to handle the logic of the try catch and simply return back an error. I don't want to be putting in try-catches all over the place; as any changes need to be done throughout the code. I think I'll just try to keep it simple and not worry about it trying to be "correct code" and work on completing the project first. – zardon Apr 15 '18 at 12:38
  • If you don't want the caller to have to say `do/catch` then have the caller use `try?` as I said before. You do this all the time when talking to Apple's code (Cocoa), so it is perfectly reasonable that a Wallet user should do it. – matt Apr 15 '18 at 12:43
  • Understood. Thank you for your assistance. I have accepted your answer. Very well described and is something for me to reflect upon, and improve. – zardon Apr 15 '18 at 12:49