[erlang-questions] Coming Back (maybe improving lists:reverse/1)

[Ivan Carmenates Garcia]

Regards,

Well seems to me that there is no more optimizations for the algorithm
(except for yours 'my_reverse_join' see below), I tested yours and mine and
they both take exactly the same for the final result the unparsed "string",
well mine is 200 millisecond faster in 1 million of iterations, yours take
64XX main 62XX approx. The problem is that using lists:flatten is the hell
slow, I also tested your algorithm just after the io lists return and it is
very faster its took 14XX milliseconds, so when you use the final
lists:flatten/1 everything goes to crap, sorry about the word. So it is
amazing who faster it is and then lists:flatten by its own take the another
5 seconds, I did knew that because I read Erlang session about 7 myths off
bla bla .. and optimizations stuffs some time ago and they say lists:flatten
is slow also I tested it when I was constructing the algorithm first time I
did avoid constructing the io lists because of that.

I did clear some things like unparse well I need another name I don't like
unparse either and parse is wrong, I will come up with something.

Also I do have something to thank you, your reverse 'my_reverse_join' little
algorithm is a crack, It sped up mine in almost one second. So no need to
use lists:reverse/1 and string:join/2, thanks for that, now it takes 54XX
instead of 62XX.

I also have questions for you if you are so kind to answer.

Regards proplists, well, it is necessary to make lists of options as
proplists? yes, proplists:compact/1 and expand/1 for [a] to [{a, true}] and
all those stuffs but what I was trying to do is to make it simple for the
user, because more tuples are more complicate to write in the case of
{users, name, alias} well that could be right adding more brackets like
{users, {name, alias}} because it organizes the thing. But i.e.: I have
another algorithm for match specs in which I can do {name | {table, name} |
value, op, name | {table, name} | value } also [{..., op, ...}, ...],
logic_op, ... so it will be easy for the user to build something like 
[
    [{id, '==', 1}, {age, '==', 31}],
    'or',
    {username, '==', "john"} 
]

And not something like
[
   {  [{id, '==', 1}, {age, '==', 31}],  'or',  {username, '==', "john"}   }
]

It was also more easy to build the algorithm for me without the tuples for
the logic operator and I also validate it easy if other term is missing by
checking if there is a logic_op with an empty Rest in the list to unparse.

Kindly regards,
Ivan (son of Gilberio).



-----Original Message-----
From: Richard A. O'Keefe [mailto:ok@REDACTED] 
Sent: Thursday, October 8, 2015 10:17 PM
To: Ivan Carmenates Garcia
Cc: Erlang Questions Mailing List
Subject: Re: [erlang-questions] Coming Back (maybe improving
lists:reverse/1)


On 8/10/2015, at 1:20 pm, Ivan Carmenates Garcia <co7eb@REDACTED> wrote:

> For example this is one of the algorithms, I optimize it as well as I
could:

Start by optimising the documentation.
>  
> Considering here that the order of the fields is very important!.
>  
> %% -------------------------------------------------------------------
> %% @private
> %% @doc
> %% Parses the specified list of full fields into a string containing 
> %% all full fields separated my comma.
> %% example:
> %% <pre>
> %%   parse_full_fields([{users, '*'}, name, {roles, [id, level]},
> %%       {users, name, alias}], fun get_postgres_operator/2) ->
> %%       {"users.'*',name,roles.id,roles.level,users.name AS alias",
> %%           [users, roles]}.
> %% </pre>
> %% Returns `{[], []}' if called with `[]'.
> %% @throws {error, invalid_return_fields_spec, InvalidForm :: any()} 
> %% @end %% 
> -------------------------------------------------------------------

Parsing turns a string into structure.
Turning structure into a string is the OPPOSITE of parsing.
You are not parsing but UNparsing.

> -spec parse_return_full_fields(FullFieldsSpecs, Separator, OperatorFinder)
-> Str when
>     FullFieldsSpecs :: proplists:proplist(),
>     Separator :: string(),
>     OperatorFinder :: fun(),
>     Str :: string().

This is seriously confusing.
The thing about a proplist, as we've recently discussed in this list, is
that it contains
 - atoms, where 'x' is equivalent to {'x',true}
 - pairs {Key, Value}
 - junk, which is completely ignored.
While I don't fully understand your example, it's clear
that    name    is NOT equivalent to {name,true} and
that    {users, name, alias}   is NOT ignored as junk.
So whatever you have, it certainly isn't a proplist.

Finally, your -spec describes a function with three arguments, but the
example in your comment has only two arguments.

It looks like you are mapping
    'atom' -> "atom"
    {'table', 'field'} -> "table.field"
    {'table', ['f1',...,'fn']} -> "table.f1" ... "table.fn"
    {'table', 'field', 'alias'} -> "table.field AS alias"

It would seem more logical to have
    field() = atom() | {atom(),atom()}.  % {name,alias}
    fields() = field() | list(field()).
    slot() = atom() | {atom(),fields()}. % {table,fields} so that it's
obvious how to put an alias inside a list of fields.

-module(goo).
-export([
    tables/1,
    unparse_return_full_fields/1,
    unparse_slots/1
 ]).

unparse_return_full_fields(Slots) ->
    {lists:flatten(unparse_slots(Slots)), tables(Slots)}.

tables([{Table,_}|Slots]) ->
    Tables = tables(Slots),
    case lists:member(Table, Tables)
      of true  -> Tables
       ; false -> [Table|Tables]
    end;
tables([Name|Slots])
  when is_atom(Name) ->
    tables(Slots);
tables([]) ->
    [].

unparse_slots([Slot|Slots]) ->
    [ unparse_slot(Slot)
    | unparse_remaining_slots(Slots) ].

unparse_remaining_slots([]) ->   
    "";
unparse_remaining_slots(Slots) ->
    ["," | unparse_slots(Slots)].

unparse_slot({Table,Fields}) ->
    unparse_fields(Fields, atom_to_list(Table));
unparse_slot(Name)
  when is_atom(Name) ->
    atom_to_list(Name).

unparse_fields([Field|Fields], Table) ->
    [ unparse_fields(Field, Table)
    | unparse_remaining_fields(Fields, Table) ];
unparse_fields({Name,Alias}, Table) ->
    [Table, ".", atom_to_list(Name), " AS ", atom_to_list(Alias)];
unparse_fields(Name, Table)
  when is_atom(Name) ->
    [Table, ".", atom_to_list(Name)].

unparse_remaining_fields([], _) ->
    "";
unparse_remaining_fields(Fields, Table) ->
    ["," | unparse_fields(Fields, Table)].

With that definition, we get

1> c(goo).
2> goo:unparse_slots([{users, '*'}, name, {roles, [id, level]},
   {users, {name, alias}}]).   %% Note difference here.[["users",".","*"],
 ",","name",",",
 [["roles",".","id"],",",["roles",".","level"]],
 ",",
 ["users",".","name"," AS ","alias"]]

This is a tree of strings, not a string.  But for many purposes, it's just
as good.  (It's called an iolist.) Flattening it gives
"users.*,name,roles.id,roles.level,users.name AS alias"
3> goo:unparse_return_full_fields([{users, '*'}, name, {roles, [id, level]},
{users, {name, alias}}]).
{"users.*,name,roles.id,roles.level,users.name AS alias",  [roles,users]}

You will note that the functions above don't have any use for reversing a
list.  And I must repeat that for many purposes, such as sending text to
another OS process, there isn't any *need* to flatten the tree of strings.

Even in your code,

> parse_return_full_fields(FullFieldsSpecs, Separator, OperatorFinder) ->
>     {ParsedFields, TableNames} =
>         parse_full_fields2(FullFieldsSpecs, [], [], OperatorFinder),
>     {string:join(lists:reverse(ParsedFields), Separator), TableNames}.

there isn't actually any need to do this:

    my_reverse_join([X|Xs], Sep) ->
        my_reverse_join(Xs, X, Sep);
    my_reverse_join([], _) ->
        "".

    my_reverse_join([], Acc, _) ->
        Acc;
    my_reverse_join([Y|Ys], Acc, Sep) ->
        my_reverse_join(Ys, Y++Sep++Acc, Sep).

9> goo:my_reverse_join(["harry","deacon","thom","avery"]).
"avery,thom,deacon,harry"


We see two ideas here which are likely to be applicable to your code in
context.

(1) It may be possible to *eliminate* a call to lists:reverse/1
    by fusing it with the function the result is being passed to.

(2) It may be possible to *eliminate* appends (++) by constructing
    an iolist (a tree of strings) instead of a string.  For example,
    io:put_chars/2 wants a unicode:chardata().
    http://www.erlang.org/doc/man/unicode.html#type-chardata
    http://www.erlang.org/doc/man/io.html#put_chars-2