CODEWITHSUNDEEP
erlang
Phil Cho
Phil Cho

Reputation: 121

Bad Record when calling list:keyfind() in erlang

I'm new to erlang, and am running into an error with records in one of my modules. I'm emulating ships inside of a shipping_state, and I want to create a simple function that will print the ship id, name, and container cap of a certain ship, based on it's ID. I utilized list:keyfind, as I believe it will help, but perhaps I am not using it correctly. I have a .hrl file that contains the record declarations, and a .erl file with the function and initialization of my #shipping_state.

shipping.erl:

-module(shipping).
-compile(export_all).
-include_lib("./shipping.hrl").

get_ship(Shipping_State, Ship_ID) ->
{id, name, containercap} = list:keyfind(Ship_ID, 1, Shipping_State#shipping_state.ships).

shipping.hrl:

 -record(ship, {id, name, container_cap}).
 -record(container, {id, weight}).
 -record(shipping_state, 
    {
      ships = [],
      containers = [],
      ports = [],
      ship_locations = [],
      ship_inventory = maps:new(),
      port_inventory = maps:new()
     }
   ).
 -record(port, {id, name, docks = [], container_cap}).

Result:

 shipping:get_ship(shipping:init(),1).
 ** exception error: {badrecord,shipping_state}
 in function  shipping:get_ship/2 (shipping.erl, line 18)

I'd like to say that keyfind should work, and perhaps when I create the tuple {id, name, containercap}, something is wrong with the syntax there, but if I need to completely rethink how I would go about doing this problem, any assistance would be greatly appreciated.

Edit, I've modified my code to follow Alexey's suggestions, however, there still appears to be the same error. Any further insights?

 get_ship(Shipping_State, Ship_ID) ->
     {ship, Id, Name, Containercap} = list:keyfind(Ship_ID, 2, 
 Shipping_State#shipping_state.ships),
     io:format("id = ~w, name = ~s, container cap = ~w",[Id, Name, Containercap]).

Upvotes: 1

Views: 1306

Answers (3)

Steve Vinoski
Steve Vinoski

Reputation: 20014

Records were added to the Erlang language because dealing with tuples fields by number was error-prone, especially as code changed during development and tuple fields were added, changed, or dropped. Don't use numbers to identify record fields, and don't treat records using their underlying tuple representation, as both work against the purpose of records and are unnecessary.

In your code, rather than using record field numbers with lists:keyfind/3, use the record names themselves. I've revised your get_ship/2 function to do this:

get_ship(Shipping_State, Ship_ID) ->
    #ship{id=ID, name=Name, container_cap=ContainerCap} = lists:keyfind(Ship_ID, #ship.id, Shipping_State#shipping_state.ships),
    io:format("id = ~w, name = ~s, container cap = ~w~n",[ID, Name, ContainerCap]).

The syntax #<record_name>.<record_field_name> provides the underlying record field number. In the lists:keyfind/3 call above, #ship.id provides the field number for the id field of the ship record. This will continue to work correctly even if you add fields to the record, and unlike a raw number it will cause a compilation error should you decide to drop that field from the record at some point.

If you load your record definitions into your shell using the rr command, you can see that #ship.id returns the expected field number:

1> rr("shipping.hrl").
[container,port,ship,shipping_state]
2> #ship.id.
2

With the additional repairs to your function above to handle the returned record correctly, it now works as expected, as this shell session shows:

3> {ok, ShippingState} = shipping:init().
{ok,{shipping_state,[{ship,1,"Santa Maria",20},
                     {ship,2,"Nina",20},
                     {ship,3,"Pinta",20},
                     {ship,4,"SS Minnow",20},
                     {ship,5,"Sir Leaks-A-Lot",20}],
                    [{container,1,200},
                     ...
4> shipping:get_ship(ShippingState, 1).
id = 1, name = Santa Maria, container cap = 20
ok

Upvotes: 4

legoscia
legoscia

Reputation: 41618

Alexey's answer answers your question, in particular the 3rd point. I just want to suggest an improvement to your keyfind call. You need to pass the tuple index to it, but you can use record syntax to get that index without hard-coding it, like this:

 list:keyfind(Ship_ID, #ship.id, Shipping_State#shipping_state.ships),

#ship.id returns the index of the id field, in this case 2. This makes it easier to read the code - no need to wonder what the constant 2 is for. Also, if for whatever reason you change the order of fields in the ship record, this code will still compile and do the right thing.

Upvotes: 2

Alexey Romanov
Alexey Romanov

Reputation: 170775

  1. See Internal Representation of Records: #ship{id=1,name="Santa Maria",container_cap=20} becomes {ship, 1, "Santa Maria", 20}, so the id is the 2nd element, not the first one. 

  2. {id, name, containercap} = ...

    should be

    #ship{id=Id, ...} = ...

    or

    {ship, Id, Name, Containercap} = ...

    Your current code would only succeed if keyfind returned a tuple of 3 atoms.

  3. The error {badrecord,shipping_state} is telling you that the code of get_ship expects its first argument to be a #shipping_state, but you pass {ok, #shipping_state{...}} (the result of init).

Upvotes: 4

Related Questions