HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Cleaning up Erlang code for a text editor

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
erlangtexteditorforcodecleaning

Problem

On Monday and Tuesday I started learning Erlang through the Erlang Koans, and around Wednesday night I decided to use my newfound knowledge to implement a simple text editor (Akin to ed(1)). Currently I have the following (also on GitLab):

```
-module(text).
-compile(export_all).

%% State => {file, buffer, cursor}
main() ->
{ok, Arg} = init:get_argument(f),
Name = lists:nth(1, lists:nth(1, Arg)),
main(Name).

main (String) ->
{ok, File} = file:open(String, [read, write]),
{ok, Buffer, Lines} = buffer(File),
io:fwrite("~B lines read of ~s~n", [Lines, String]),
loop({File, Buffer, 1}).

loop(State) ->
NewState = command(State),
case NewState of
{_, _, _} -> loop(NewState);
_ -> ok
end.

append(Buffer, Line) ->
erlang:insert_element(tuple_size(Buffer)+1, Buffer, Line).

buffer(File) ->
buffer(File, {}).
buffer(File, Buffer) ->
Line = io:get_line(File, ""),
case Line of
eof -> {ok, Buffer, tuple_size(Buffer)};
{error, Description} -> {error, Description};
_ -> buffer(File, append(Buffer, Line))
end.

put(stdout, {_, {}, _}, _) ->
'nothing to write';
put(stdout, {_, Buffer, _}, Line) ->
io:fwrite(" ~s", [element(Line, Buffer)]);
put(file, {File, Buffer, _}, Line) ->
io:fwrite(File, "~s", [element(Line, Buffer)]).

%% Ew Ew Ew Ew Ew
write_buffer(stdout, {File, Buffer, Pos}) ->
write_buffer(stdout, {File, Buffer, Pos}, 1, tuple_size(Buffer)).
write_buffer({File, Buffer, Pos}) ->
file:position(File, bof),
write_buffer(file, {File, Buffer, Pos}, 1, tuple_size(Buffer)).

write_buffer(Out, State, N, Length) when N =
put(Out, State, N),
write_buffer(Out, State, N+1, Length);
write_buffer(_, {File, {}, Pos}, _, _) ->
file:truncate(File),
{File, {}, Pos};
write_buffer(_, State, _, _) ->
State.

put_cursor(N, {File, Buffer, Pos}) ->
put_cursor(Pos, Pos+N, {File, Buffer, Pos}).
put_cursor(Current, End, State) when Current
put(stdout, State, Current),
put_cursor(Current+1, End, State);
put_cursor(_,

Solution

What should the code do?

Below are some corrections that I would do:

.1. Instead of:

{ok, Arg} = init:get_argument(f),
Name = lists:nth(1, lists:nth(1, Arg)),
main(Name).


Do:

{ok, [[Name, _], _]} = init:get_argument(f),
main(Name).


Does this fragment work correctly?

.2. Instead of:

modify(replace, State) ->
  modify(replace, State, input());
modify(append, State) ->
  modify(append, State, input());
modify(change, State) ->
  modify(change, State, input()).


Do:

modify(Action, State) ->
  modify(Action, State, input()).


.3. Instead of:

delete([], {File, Buffer, Pos}) ->
  remove(Pos, {File, Buffer, Pos});
delete([Char|Tail], {File, Buffer, Pos}) ->
  case Char of
    %% *,
    42 -> {File, {}, Pos};
    _  -> remove(get_arg([Char] ++ Tail), {File, Buffer, Pos})
  end;
delete(N, {File, Buffer, Pos}) ->
  delete(Pos, Pos+N, {File, Buffer, Pos}).


Do:

delete([], {File, Buffer, Pos}) ->
  remove(Pos, {File, Buffer, Pos});
delete([42 = Char|Tail], {File, Buffer, Pos}) ->
  {File, {}, Pos};
delete([Char|Tail], {File, Buffer, Pos}) ->
  remove(get_arg([Char] ++ Tail), {File, Buffer, Pos});
delete(N, {File, Buffer, Pos}) ->
  delete(Pos, Pos+N, {File, Buffer, Pos}).


Otherwise the code looks quite OK. If it looks repetitive then it's probably because the State and common tuples are being dragged along to all the functions. Some of it could also be down to the choice of data structures used to store data, i.e. why Buffer is a tuple and not a list? In Erlang it's much easier to manipulate variable-size data with lists than with tuples.

Code Snippets

{ok, Arg} = init:get_argument(f),
Name = lists:nth(1, lists:nth(1, Arg)),
main(Name).
{ok, [[Name, _], _]} = init:get_argument(f),
main(Name).
modify(replace, State) ->
  modify(replace, State, input());
modify(append, State) ->
  modify(append, State, input());
modify(change, State) ->
  modify(change, State, input()).
modify(Action, State) ->
  modify(Action, State, input()).
delete([], {File, Buffer, Pos}) ->
  remove(Pos, {File, Buffer, Pos});
delete([Char|Tail], {File, Buffer, Pos}) ->
  case Char of
    %% *,
    42 -> {File, {}, Pos};
    _  -> remove(get_arg([Char] ++ Tail), {File, Buffer, Pos})
  end;
delete(N, {File, Buffer, Pos}) ->
  delete(Pos, Pos+N, {File, Buffer, Pos}).

Context

StackExchange Code Review Q#130065, answer score: 2

Revisions (0)

No revisions yet.