Return to Revisions

This seems closely tied to your other question. I posted an answer for that one, which I'll just incorporate 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" variable (i.e. the closed-over balance argument)

If it's not on purpose, and you'd be ok with having balance be publicly accessible, you could just use CoffeeScript's class syntax.

But, assuming you do want the "private" balance variable, there are still 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 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 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 fetch 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 descriptive.

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 then 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.