5
\$\begingroup\$

I have the following task:

Create a simple treasure hunt game.

Create a two-dimensional array of integers 10 by 10. In a random position in the array store the number 1. repeat Get the user to enter coordinates where they think the treasure is. If there is a 1 at this position display ‘success’. Until they find the treasure

Instead of using arrays I used lists:

-module(hunt).
-export([start/0]).
start() -> 
    X = [[12,1,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5]],
    loop(X).
loop(X) ->
    Inputs = io:get_line("Enter the coordinates"),
    {A,_} = string:to_integer(string:substr(Inputs,1,2)),
    {B,_} = string:to_integer(string:substr(Inputs,3,2)),
    erlang:display(A),
    erlang:display(B),
    case get(X,A,B) of
        1 -> io:format("You did it!");
        _ -> io:format("Try again"), loop(X)
    end.
get(List,A,B) ->
    ghelper(List,A,B).
ghelper(List,A,B) -> ghelper(List,A,B,0,0).
ghelper([H|Tail],A,B,N,M) -> 
    case N == A of
        true -> ghelper(second,H,B,M);
        false -> ghelper(Tail,A,B,N+1,M)
    end.
ghelper(second,[H|Tail],B,M) -> 
    case M == B of
        true -> H;
        false -> ghelper(second,Tail,B,M+1)
    end.

Is there a way to improve this further? I know I could probably make a function to randomly generate a 2D array (represented by lists here), but apart from that, is there anything I could improve?

\$\endgroup\$
3
\$\begingroup\$

Even though your code works I have some comments regarding Erlang style and naming conventions:

  1. Spaces after commas and empty lines between function definitions make the code a lot more readable.
  2. Helper functions that do the actual work are usually named with the do_ prefix. Instead of ghelper I would have named that do_get.
  3. Using lists:nth could greatly simplify your code. The standard library includes a lot of useful functions, I try to always check if there is an existing function for what I want to do and only if there's none then I implement it.
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$
  • You should clean up your output strings, since currently they print without any punctuation or newlines and so make it hard for the user to read them.
  • Avoid using get as a function name, since there's already a get function in the erlang module, and experienced Erlang programmers will initially assume you're using that function rather than your own local function. I renamed your function to element.
  • You don't specify whether the coordinates you're requesting from the user are 0-based or 1-based. Running your program shows they're 0-based, which is fine, but you need to be clear about that.
  • There's really no need for a function named ghelper; you could instead just use different clauses for the element function. Or better yet, just use lists:nth/2 as described in another answer and get rid of all the other clauses entirely.
  • Use io:fread/2 to get the coordinates from the user, since it can read the inputs and convert them to integers in one step.
  • You need to check that the values the user enters are within range.
  • Use more descriptive variable names.

Here's a revised version that takes this advice into account:

-module(hunt).
-export([start/0]).

start() ->
    Values = [[12, 1,14,15,16,22,11,11,2,5], [12,13,14,15,16,22,11,11,2,5],
              [12,13,14,15,16,22,11,11,2,5], [12,13,14,15,16,22,11,11,2,5],
              [12,13,14,15,16,22,11,11,2,5], [12,13,14,15,16,22,11,11,2,5],
              [12,13,14,15,16,22,11,11,2,5], [12,13,14,15,16,22,11,11,2,5],
              [12,13,14,15,16,22,11,11,2,5], [12,13,14,15,16,22,11,11,2,5]],
    io:format("Guess the 0-based coordinates of the value 1.\n"),
    loop(Values, length(Values), length(hd(Values))).

loop(Values, RowMax, ColMax) ->
    {ok, [Row,Col]} = io:fread("Guess the row and column, comma-separated: ",
                               "~d,~d"),
    case {Row < RowMax, Col < ColMax} of
        {true, true} ->
            case element(Values, Row, Col) of
                1 ->
                    io:format("Congratulations, you found the 1 at (~w,~w)!\n",
                              [Row, Col]);
                _ ->
                    io:format("The 1 is not at (~w,~w), please try again\n",
                              [Row, Col]),
                    loop(Values, RowMax, ColMax)
            end;
        _ ->
            io:format("(~w,~w) is out of range, please try again\n", [Row, Col]),
            loop(Values, RowMax, ColMax)
    end.

element(Values, Row, Col) ->
    lists:nth(Col+1, lists:nth(Row+1, Values)).

And here's example output from compiling and running it:

$ erlc hunt.erl
$ erl -noshell -s hunt -s init stop
Guess the 0-based coordinates of the value 1.
Guess the row and column, comma-separated: 3,4
The 1 is not at (3,4), please try again
Guess the row and column, comma-separated: 10,11
(10,11) is out of range, please try again
Guess the row and column, comma-separated: 0,1
Congratulations, you found the 1 at (0,1)!
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I read your code and it works. I have some suggestions though, that could apply to a more complex example.

  • I think that you should dig in the libraries in order to use existing functions. In my opinion it helps for readability and generally it saves some line of code: less work, less bugs, more time to spend for the user.

  • In this example, the code interacts with the user, its is a case where the "let it crash" erlang habit does not suit the usage,
    I suggest you to use the io_lib to read the expected input from the user, and ind him the syntax in case of error (You could also have a give up option).

  • I prefer to use different function heads (pattern matching) rather than case statement to deal with recursion stop condition, for example the code

    ghelper([H|Tail],A,B,N,M) -> case N == A of true -> ghelper(second,H,B,M); false -> ghelper(Tail,A,B,N+1,M) end.

may be replaced by:

ghelper([H|Tail],A,B,A,M) -> 
        ghelper(second,H,B,M);
ghelper([H|Tail],A,B,N,M) -> 
        ghelper(Tail,A,B,N+1,M).

and even better using explicit names:

get_row([H|Tail],A,B,A) -> 
        get_cell(H,B,1); // start at 1 to give the same result as lists:nth/2
get_row([H|Tail],A,B,N) -> 
        get_row(Tail,A,B,N+1).

But this version fails if the user enters a bad syntax, or out of boundary coordinates, I propose this version which checks the user input. The answer decoding and checking are straight, half of the code is for the user friendliness

-module(hunt).

-export([start/0,start/2]).

start() -> start(10,10).
start(MAXCOL,MAXROW) ->  
    loop(generate(MAXROW,MAXCOL),MAXROW,MAXCOL).

loop(X,MAXROW,MAXCOL) ->
    Inputs = io:get_line("Enter the coordinates : "),
    case check(io_lib:fread("~d~d",Inputs),X,MAXROW,MAXCOL) of
        true -> io:format("got it~n");
        _ -> io:format("try again~n"),
                 loop(X,MAXROW,MAXCOL)
    end.

check({ok,[A,B],_},X,MAXROW,MAXCOL) when A > 0, B > 0, A =< MAXCOL , B =< MAXROW ->
    lists:nth(A,lists:nth(B,X)) =:= 1;
check(_,_,MAXROW,MAXCOL) -> 
    io:format("Invalid answer , please give an answer formatted as : \"X Y\"~nwhere 0 < X <= ~p, 0 < Y <= ~p~n",
        [MAXCOL,MAXROW]).

generate(MAXROW,MAXCOL) ->
    random:seed(erlang:timestamp()),
    Xs = random:uniform(MAXCOL),
    Ys = random:uniform(MAXROW),
    F = fun(X,Y) when X == Xs, Y == Ys -> 1; (_,_) -> random:uniform(10) + 1 end,
    Res = [[F(X,Y) || X <- lists:seq(1,MAXCOL)] || Y <- lists:seq(1,MAXROW)],
    io:format("~p~n",[Res]),
    Res.

compile and test it

1> c(hunt).
{ok,hunt}
2> hunt:start(8,3).
[[6,11,9,8,7,6,8,7],[3,4,2,2,4,4,5,6],[2,8,2,6,5,8,1,11]]
Enter the coordinates : hello!
Invalid answer , please give an answer formatted as : "X Y"
where 0 < X <= 8, 0 < Y <= 3
try again
Enter the coordinates : 1 2
try again
Enter the coordinates : hello
Invalid answer , please give an answer formatted as : "X Y"
where 0 < X <= 8, 0 < Y <= 3
try again
Enter the coordinates : 8 3
try again
Enter the coordinates : 1 1
try again
Enter the coordinates : 8 4
Invalid answer , please give an answer formatted as : "X Y"
where 0 < X <= 8, 0 < Y <= 3
try again
Enter the coordinates : 7 3
got it
ok
3>
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.