[erlang-questions] feedback please

Richard A. O'Keefe ok@REDACTED
Fri Sep 25 02:50:41 CEST 2015


First, let's clean up the code you posted,
which seems to have been garbled a bit in transmission.

-module(balance).
-export([get_balance/1]).

get_balance(Balance)  ->
    io:format("~n~n"),  
    io:format("Your account balance is $~w.~n",  [Balance]),
    Entered_action = io:get_line(
        "Which Action do you need (w)ithdraw/(d)eposit? "),
    Action = string:strip(Entered_action, right, $\n),
    Entered_amount = io:get_line(
        "What Amount do you want to withdraw/deposit? "),
    Amount2 = string:to_integer(string:strip(Entered_amount, right, $\n)),    
    Amount = element(1, Amount2), 
    get_balance(Balance, Action, Amount).    

get_balance(0, "w", _) ->    
    io:format("You have no funds to withdraw.~n"),
    get_balance(0);
get_balance(Balance, "w", Amount) 
  when Amount > Balance ->    
    io:format("You cannot withdraw more than you have.~n"),
    get_balance(Balance);  
get_balance(Balance, "w", Amount) ->     
    get_balance(Balance - Amount);    
get_balance(Balance, "d", Amount) ->  
    get_balance(Balance + Amount).


Second, let's look at the names.

What does "get" mean?
NOTHING.
So why is it even there?
If anything "get_balance" implies something that *reports*
a balance *without changing it*, which makes "get" worse
than useless.

Call it something like "maintain_balance/1."

Now, let's look at your sanity checking.
- You ensure that the *change* to a balance is an integer,
  but you nowhere check that the *initial* balance is one.
- You check that someone doesn't withdraw more than they
  have, but you do not check that they do not deposit a
  large negative amount.


Your account balance is $1050.
Which Action do you need (w)ithdraw/(d)eposit? d
What Amount do you want to withdraw/deposit? -2000

Finally, it's nice to be able to stop the loop without having
to crash it.

There are several further improvements possible,
but at this point here's what we get:

-module(balance).
-export([maintain_balance/1]).

maintain_balance(Initial_Balance)
  when is_integer(Initial_Balance), Initial_Balance >= 0 ->
    loop(Initial_Balance).

loop(Balance)  ->
    io:format("~nYour account balance is $~w.~n", [Balance]),
    case prompted_character(
         "Which Action do you need (w)ithdraw/(d)eposit/(q)uit? ")
      of $q -> Balance
       ; $w -> Amount = prompted_integer(
                   "What amount do you want to withdraw? "),
               loop(withdraw(Balance, Amount))
       ; $d -> Amount = prompted_integer(
                   "What amount do you want to deposit? "),
               loop(deposit(Balance, Amount))
    end.

prompted_character(Prompt) ->
    hd(io:get_line(Prompt)).

prompted_integer(Prompt) ->
    Amount = list_to_integer(string:strip(io:get_line(Prompt), right, $\n)),
    if Amount <  0 -> io:format("Amounts should not be negative.~n"), 0
     ; Amount >= 0 -> Amount
    end.

withdraw(Balance, Amount)
  when Balance >= Amount ->
    Balance - Amount;
withdraw(Balance, _) ->
    io:format("Insufficient funds.~n"),
    Balance.

deposit(Balance, Amount) ->
    Balance + Amount.





More information about the erlang-questions mailing list