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

[Ivan Carmenates Garcia]

Well, using this function I write instead of 'lists:flatten/1' your
algorithm takes 50XX instead of 62XX milliseconds when using
'lists:flatten/1'.

But if instead of adding the "," in your iolist generator algorithm we do it
here somehow it takes about 42XX milliseconds, I did but I still missing one
comma. I get something like this
"users.*,name,roles.id,roles.levelusers.name AS alias, comma missing between
role.level and user.name. I think we have no control of that in
'concat_io_list/2' function.

 

concat_io_list([], Acc) ->

    Acc;

concat_io_list([[L | _] = List | Rest], Acc) when is_list(L)->

    concat_io_list(Rest, Acc ++ join_simple_list(List, []));

concat_io_list([List | Rest], Acc) ->

    concat_io_list(Rest, Acc ++ List).

 

join_simple_list([], Acc) ->

    Acc;

join_simple_list([[X | _] = Str | Xs], Acc) when is_number(X) ->

    join_simple_list(Xs, Acc ++ Str);

join_simple_list([Str | Xs], Acc) ->

    join_simple_list(Xs, Acc ++ join_simple_list(Str, [])).

 

Well thanks for all,

Best 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 <
<mailto:co7eb@REDACTED> 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/unicode.html#type-chardata

     <http://www.erlang.org/doc/man/io.html#put_chars-2>
http://www.erlang.org/doc/man/io.html#put_chars-2

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20151009/d47cafaa/attachment.htm>