basho / cuttlefish

never lose your childlike sense of wonder baby cuttlefish, promise me?
Apache License 2.0
205 stars 124 forks source link

Enums: Should they be more like flags? #130

Closed joedevivo closed 10 years ago

joedevivo commented 10 years ago

Imagine

%% @doc Specifies the storage engine used for Riak's key-value data
%% and secondary indexes (if supported).
{mapping, "storage_backend", "riak_kv.storage_backend", [
  {default, {{storage_backend}} },
  {datatype, {enum, [bitcask, leveldb, memory, multi]}}
]}.

{translation,
 "riak_kv.storage_backend",
 fun(Conf) ->
    Setting = cuttlefish:conf_get("storage_backend", Conf),
    case Setting of
      bitcask -> riak_kv_bitcask_backend;
      leveldb -> riak_kv_eleveldb_backend;
      memory -> riak_kv_memory_backend;
      multi -> riak_kv_multi_backend;
      _Default -> riak_kv_bitcask_backend
    end
 end}.

Now imagine if it were just:

%% @doc Specifies the storage engine used for Riak's key-value data
%% and secondary indexes (if supported).
{mapping, "storage_backend", "riak_kv.storage_backend", [
  {default, {{storage_backend}} },
  {datatype, {enum, [
      {bitcask, riak_kv_bitcask_backend}, 
      {leveldb, riak_kv_eleveldb_backend}, 
      {memory, riak_kv_memory_backend}, 
      {multi, riak_kv_multi_backend}
    ]}}
]}.
seancribbs commented 10 years ago

Reminder from our discussion: Having enums like this will eliminate the need for the {flag, {OnName, OnValue}, {OffName, OffValue}} form of flags, since we're not talking about a boolean thing anymore; so we could potentially delete code there.

joedevivo commented 10 years ago

In progress, check the branch jd-better-enum

joedevivo commented 10 years ago

Addresses the configuration issue described in basho/riak#515

seancribbs commented 10 years ago

Address the comments I left and I'll give the thumbs-up. This is a great improvement.

seancribbs commented 10 years ago

Still getting this warning (should be io:format/0):

Unknown types:
  io_lib:format/0
seancribbs commented 10 years ago

:+1: 86168f1

seancribbs commented 10 years ago

@borshop :+1: 86168f1

joedevivo commented 10 years ago

@borshop merge 86168f1