nim-lang / RFCs

A repository for your Nim proposals.
137 stars 23 forks source link

Add a proc to return an Option[T] from a table #351

Open ghost opened 3 years ago

ghost commented 3 years ago

Proposal

Add a proc like proc getOpt*[A, B](t: Table[A, B], key: A): Option[B] to the tables stdlib module.

Example

We have a Table[string, float] and want to check if a value is in the table, and if it is, get that value. The easiest solution is:

if "key" in table:
  table["key"]

There's one issue with this code, though - it has to do a lookup for the key twice (one for hasKey, one for []). While it's not something that's particularly bad, sometimes it might be desirable to only do one lookup.

Another option is getOrDefault, but then we'll have to pick some arbitrary default value that might exist in the table already (e.g., if the user of the library can provide their own custom values). So, while it can work in some cases, it won't work in all of them.

The final option is to use [] and catch KeyError if the key is not in the table. That'll only do 1 lookup, but I'm not sure if using exceptions is worth it in this example.

Proposed solution

Add a proc proc getOpt*[A, B](t: Table[A, B], key: A): Option[B] (and overloads for TableRef and other table types) to the tables module to allow getting a value from a table with a single lookup.

Advantages

Disadvantages

I thought about making a tableutils module, but that wouldn't be possible since the data field of the Table is not exported, and tableimpl only has templates that other modules can't use to access the data field.

Alternatives

If the additional options module is too much, we can do it the tuple way with proc getOpt[A, B](t: Table[A, B], key: A): (bool, B), albeit that's a bit more "raw".

Implementation

Quite simple (assuming adding to the tables module):

import std/options
proc getOpt*[A, B](t: Table[A, B], key: A): Option[B] = 
  ## Returns some(value) if `key` is in the table, otherwise returns `none()`.
  var hc: Hash
  let index = rawGet(t, key, hc)
  if index >= 0:
    result = some(t.data[index].val)

Relevant discussions

ghost commented 3 years ago

I found out that there's a withValue template in tables that can be used to create a similar proc in a separate module, but it requires a mutable Table and I'm not particularly fond of one-proc modules in the stdlib. And even then tableutils won't be able to grow since we wouldn't be able to access internal Table fields anyway.

konsumlamm commented 3 years ago

I like the idea in general, but what about a mutable version? I'm not sure it's possible though (my attempt gives a C error).

Perhaps an alternative is to provide a version of withValue that doesn't require var? Then getOpt could just be implemented in another module or a 3rd party package.

ghost commented 3 years ago

@konsumlamm having a mutable table value with an Option type is not safe unless with view types, and they're experimental

ghost commented 3 years ago

As discussed in the meeting, one problem is the mutability - right now you can't really get a safe immutable view with Option[T]. Another problem is about consistency - if we want to add a new API to tables then we'll have to do the same to other collection types.

Araq commented 3 years ago

First we should get view types into the state where Option[var T] and Option[lent T] do work reliably and then this RFC has my blessing.

dom96 commented 3 years ago

I support this, but in the words of Justin Timberlake: drop the Opt, it's cleaner :)