Implementing a Simple "MoneyAccount" in CoffeeScript

Question

Please critique this CoffeeScript code. Is it idiomatic?

MoneyAccount = (balance = 0) ->
 obj = {
 withdraw: (x) ->
 if balance < x
 throw new Error("not enough funds in account")
 else
 balance -= x
 console.log "withdrew funds."
 x
 deposit: (x) ->
 if (typeof x isnt 'number' || x < 0)
 throw new Error("bad deposit amount " + x)
 else
 balance += x
 printBalance: ->
 console.log balance
 }
 obj

Answer 1

This seems closely tied to your other question. I posted an answer for that one, which I'll incorporate/reference here.

Calling it MoneyAccount (in "UpperCamelCase") implies that it's a constructor. But it's not quite; it's just a function. But I assume that's done on purpose in order to have "private" variables (i.e. the closed-over balance argument) while still allowing one to invoke it with or without the new keyword.

Still, there are a couple of things you could do:

1. You're diligent in checking the input for deposit and withdraw... but not the initial balance. So account = MoneyAccount("foo") would spell trouble right away.
   But even if you check the type of the initial balance, it'd still allow someone input a negative initial balance. But the way withdraw works seems to imply that a negative balance is impossible.
   Incidentally, I could also say account = MoneyAccount(1/0) and have Infinity money! Sounds nice, but perhaps not what you want.

2. Similar to the above, there's nothing stopping you from withdrawing or depositing a negative or infinite amount.

3. As mentioned in the answer I linked to, you don't need if...else branches if you throw an an exception; the function will exit if it throws.

4. There's no way to just get the balance, which would seem problematic for an account. You can print it, sure, but you can't just get it. If you add a method to do that, you can skip the console.log stuff (which incidentally, you're only doing for withdraw but not for deposit). Logging stuff isn't really the responsibility of an account object anyway; leave that to external code.

5. x should probably be named amount instead, just to be descriptive.

6. You might want to throw a RangeError in withdraw to be a bit more precise.

7. Return values are a little haphazard. deposit returns the new balance while withdraw returns the amount withdrawn.

8. The obj var is unnecessary; you can just leave it out entirely. And you can even leave out the curly brackets (which I personally like to do in a case like this, but it's a matter to taste, really).

In the end, I get this:

MoneyAccount = (balance = 0) ->
 # Adapted from the linked answer
 assertValidAmount = (amount) ->
 number = Number amount
 if isNaN(number)
 throw new TypeError("Expected a numeric amount")
 if not isFinite(number) or number < 0
 throw new RangeError("Expected a positive, finite amount")
 assertValidAmount balance
 # everything below will be interpreted as an object
 # and returned, even without the curly braces
 balance: -> balance
 withdraw: (amount) ->
 assertValidAmount amount
 throw new RangeError("Insufficient funds") if amount > balance
 balance -= amount
 deposit: (amount) ->
 assertValidAmount amount
 balance += amount

That should be pretty safe.

• Kevin Meredith: I used the Function rather than class due to CoffeeScript Ristretto's examples. With respect to If it's not on purpose, and you'd be ok with having balance be publicly accessible, how can that ever be acceptable? Thanks for your informative, detailed answer. I appreciate it.
• Flambino: @KevinMeredith I agree that a publicly accessible balance sounds like a bad idea - I just mentioned it because that'd be the consequence of using CoffeeScript's class. But in the end, JS doesn't have a built-in concept of private/public visibility or accessibility (e.g. you can redefine the functions and properties on existing prototypes), so it's often times easier to just leave things loose than erect and maintain a bunch of "artificial" barriers (similar to GIGO vs exceptions). Closed-over "private" vars are however simple to do, so that's encouraged!
• Kevin Meredith: If class MoneyAccount were used instead, then couldn't we always use a normal variable like so: class Queue foo = "Hey there" constructor: -> ...? When constructing a new object, foo wouldn't be accessible from without the class MoneyAccount, if I understand correctly. Example - gist.github.com/kman007us/18dd99f8962c88a874ae
• Flambino: @KevinMeredith Ah, of course, you are correct. My mistake; I was misremembering the syntax. I've edited the answer. Thanks for the correction