compilation error: record replacement of macro in Erlang

Question

This is the directory structure.

src/
animal.hrl
people.hrl
data_animal.erl
data_people.erl
test.erl
test_macro.erl

animal.hrl

%% The record definition of animal.

-ifndef(ANIMAL).
-define(ANIMAL,true).

-record(animal,{
  id,
  animal_name,
  age
}).

-endif.

people.hrl

%% The record definition of people.

-ifndef(PEOPLE).
-define(PEOPLE,true).

-record(people,{
  id,
  people_name,
  age
}).

-endif.

data_animal.erl

%% The data file of animal.

-module(data_animal).
-include("animal.hrl").

%% API
-export([get/1,get_ids/0]).

get(1)->
  #animal{
    id=1,
    animal_name="cat",
    age=23
  };
get(2)->
  #animal{
    id=2,
    animal_name="dog",
    age=19
  };
get(3)->
  #animal{
    id=3,
    animal_name="tiger",
    age=23
  };
get(4)->
  #animal{
    id=4,
    animal_name="pig",
    age=19
  };
get(_)->
  undefined.

get_ids()->
  [1,2,3,4].

data_people.erl

%% The data file of people.

-module(data_people).
-include("people.hrl").

%% API
-export([get/1,get_ids/0]).

get(1)->
  #people{
    id=1,
    people_name="John",
    age=23
  };
get(2)->
  #people{
    id=2,
    people_name="Ken",
    age=19
  };
get(3)->
  #people{
    id=3,
    people_name="Tom",
    age=23
  };
get(4)->
  #people{
    id=4,
    people_name="Healthy",
    age=19
  };
get(_)->
  undefined.

get_ids()->
  [1,2,3,4].

Notice that, for data_animal.erl and data_people.erl, the parameter of get/1is the record's id of the return value, and the return value of get_ids/0 is a list of get/1's parameters.

test.erl

-module(test).

%% API
-export([get_animal_list/1,get_people_list/1]).

-include("animal.hrl").
-include("people.hrl").

get_animal_list(Age)->
  Fun=fun(Id,Acc)->
    case data_animal:get(Id) of
      #animal{age=Age}=Conf->
        [Conf|Acc];
      _->
        Acc
    end
      end,
  lists:foldl(Fun,[],data_animal:get_ids()).

get_people_list(Age)->
  Fun=fun(Id,Acc)->
    case data_people:get(Id) of
      #people{age=Age}=Conf->
        [Conf|Acc];
      _->
        Acc
    end
      end,
  lists:foldl(Fun,[],data_people:get_ids()).

I want to get the data of animal and people, whose ages are 23, so I write 2 functions, get_animal_list/1, get_people_list/1.

I run

1> c(data_animal),c(data_people),c(test).
{ok,test}
2> test:get_people_list(23).
[{people,3,"Tom",23},{people,1,"John",23}]
3> test:get_animal_list(23).
[{animal,3,"tiger",23},{animal,1,"cat",23}]

Suddenly, I find that the 2 functions share the same pattern. Then I attempt to write a macro get_list, and make 2 calls instead.

test_macro.erl

-module(test_macro).

%% API
-export([get_animal_list/1,get_people_list/1]).

-include("animal.hrl").
-include("people.hrl").

-define(get_list(DataMod,Record,Age),(
  Fun=fun(Id,Acc)->
    case DataMod:get(Id) of
      #Record{age=Age}=Conf->
        [Conf|Acc];
      _->
        Acc
    end
  end,
  lists:foldl(Fun,[],DataMod:get_ids())
)).

get_animal_list(Age)->
  ?get_list(data_animal,animal,Age).

get_people_list(Age)->
  ?get_list(data_people,people,Age).

But I got the compile error:

4> c(test_macro).
test_macro.erl:22: syntax error before: ','
test_macro.erl:25: syntax error before: ','
test_macro.erl:4: function get_animal_list/1 undefined
test_macro.erl:4: function get_people_list/1 undefined
error

Tell me why~y~y~

Thank you all! I have 3 questions now.

1. Is my code really not Erlang-like? It's extracted from my company's project. Am I still thinking in OOP? Or so do the programming guys in my company?
2. Thanks to @mlambrichs 's advice. It works, but I still wonder why my code get the compilation error? Is it because Erlang preprocessor is a one-pass scanner, so it fails to recognize #Record{age=Age}?

3. According to @mlambrichs 's suggestion, I try to change the macro

   -define(get_list(DataMod, Record, Age), [P || P <- lists:map(fun(Id) -> DataMod:get(Id) end, DataMod:get_ids()), P#Record.age =:= Age] ).

into a function

get_list(DataMod, Record, Age)->
  [P || P <- lists:map(fun(Id) -> DataMod:get(Id) end,
    DataMod:get_ids()),
    P#Record.age =:= Age].

Then I get the compilation error: syntax error before: Record

Answer 1

The cause of the error is a misplaced '(' which should be removed:

-define(get_list(DataMod,Record,Age), (
                                     ^^^
   Fun=fun(Id,Acc)->              
     case DataMod:get(Id) of
        #Record{age=Age}=Conf->
           [Conf|Acc];
        _->
           Acc
     end
   end,
   lists:foldl(Fun,[],DataMod:get_ids())
).

EDIT You added some questions which I would like to start answering now.

• Is my code really not Erlang-like?
  • Usage of macros. There's no real need to use macros in your situation.
  • In general: you would like to hide the fact what kind of records are used in people and animals. That's implementation and should be shielded by your interface. You can just define a getter function that takes care of that in the right module. Pls. read my rewrite suggestions.
• It works, but I still wonder why my code get the compilation error? See top of answer.
• ....I try to change the macro .... You're right, that function doesn't compile. Apparently a rewrite is needed.

Like this:

get_list(DataMod, Age) ->
    [ P || P = {_,_,_,A} <- lists:map(fun(Id) -> DataMod:get(Id) end,
           DataMod:get_ids()),
           A =:= Age].

EDIT

Taking up a rewrite. What you want is a concatenation of two list in function test (yours test/0, mine test/1). Using a comma doesn't do that for you. ;-)

test(X)->
  ?get_list(data_animal,X) ++
  ?get_list(data_people,X).

Let's fix that get_list macro as well. Your macro definition get_list has 3 parameters, where it only needs 2. Why use Record as a parameter when you already use that in get_people_list/1 and get_animal_list/1? For example, try this:

-define(get_list(DataMod, Y),
   case DataMod of
      data_animal -> get_animal_list(Y);
      data_people -> get_people_list(Y);
      _Else -> []
   end
)

Overall, there is a lot of code replication in your test module. As a follow up to @yjcdll's advice, move the interface functions to animal and people to their own modules.

Let's have a look at your data modules and their get functions, as well.

I would suggest putting all people records in an array, in your case in the data_people module.

people() -> [
   #people{ id=1, people_name="John", age=23 },
   #people{ id=2, people_name="Ken", age=19 },
   #people{ id=3, people_name="Tom", age=23 },
   #people{ id=4, people_name="Healthy", age=19 } ].

Next, you would need a getter function to get only the people with a certain age:

get(Age) ->
   [X || X <- people(), is_age( X, Age )].

And the is_age/2 function would be:

is_age( Person, Age ) ->
   Person#people.age =:= Age.

So in module test your get_people_list/1 would get a lot simpler.

get_people_list(Age) ->
   data_people:get(Age).

And so on. Always be on the lookout for code that looks pretty much the same like code you've already used somewhere. Just try to behave as a sane, lazy programmer. Lazy = good. ;-)

EDIT: OP has to stick to modules given. So a rewrite of the macro is:

-define(get_list(DataMod, Record, Age),
   [P || P <- lists:map(fun(Id) -> DataMod:get(Id) end, 
                                   DataMod:get_ids()), 
         P#Record.age =:= Age]
).