motoko-base icon indicating copy to clipboard operation
motoko-base copied to clipboard

a `StableHashMap` module.

Open matthewhammer opened this issue 3 years ago • 7 comments

An outgrowth the the discussion in #299

A non-OO, stable version of our (currently OO) HashMap implementation.

matthewhammer avatar Nov 03 '21 17:11 matthewhammer

BTW, I wanted to add @nomeata as I reviewer on this one and #299 but the GH UI does not permit it. I guess he needs to be added to a group of potential reviewers somewhere in the admin settings for the repo?

matthewhammer avatar Nov 03 '21 22:11 matthewhammer

Yeah, you can only assign to people having push permission. I guess not having to review is a benefit of not being able to commit :-)

nomeata avatar Nov 03 '21 22:11 nomeata

Shouldn't we finish the discussion first whether the idiom of “class interface backed by stable data” is good enough? That would not need a new module and could even be backwards compatible with the existing interface. Maybe I need to create a PR to explain.

nomeata avatar Nov 03 '21 22:11 nomeata

I think I’d prefer the idiom described in #299 (comment) over adding new modules or classes.

I also see the benefits of not adding these to master. I favor @nomeata's alternative approach, al things considered, assuming we can reach consensus about it.

If so, then this PR is here because

  • Potentially unblocking anyone "out there" in the greater Motoko/IC land who wants this abstraction now for whatever reason. I wanted to demonstrate these self-contained versions of the Stable versions for them.
  • Support the larger stable vs OO discussion, by showing another point in the design space.

Of course, each Stable version duplicates a lot of code, which is not very compelling or satisfying, and ultimately why we should not use them.

matthewhammer avatar Nov 05 '21 17:11 matthewhammer

impossible to perform relevant operations on an actual stable hash-map ... What would I actually do with a plain HashMap?

I admit that the API for that type is limited.

One could fill a Buffer with the contents efficiently, via

  • size (to pre-allocate that Buffer of the right size) and
  • entries, to iterate over the elements.

But really, my full intention is to refactor this to be more layered, like @nomeata's suggestion, where the stable part can go into a stable variable. I need to refactor this code for that to work, I realize.

... which I can't even use get on?

Well, get requires key equality and hashing, so you need to have the non-stable HashMap_ version to project specific keys, and call those operations.

matthewhammer avatar Nov 05 '21 18:11 matthewhammer

Well, get requires key equality and hashing, so you need to have the non-stable HashMap_ version to project specific keys, and call those operations.

Sure, but that makes HashMap a useless type, doesn't it? Yes, I can now define a stable variable of this type, but I can neither look up anything nor modify it ever again. I can convert it to a list of entries, but if that's all I wanted then I could just as well have stored it as a plain list or array right away.

rossberg avatar Nov 08 '21 10:11 rossberg

@rossberg Yes, the type definitions you saw here are not as useful as the one where the types are "layered", rather than flattened, as I mentioned above:

But really, my full intention is to refactor this to be more layered, like @nomeata's suggestion, where the stable part can go into a stable variable. I need to refactor this code for that to work, I realize.

It was my original intention to do this layering, but I first tried this variation, which I agree is not as helpful.

As for the "original idea", see this commit: https://github.com/dfinity/motoko-base/pull/300/commits/e7a2787c66f132237bb3f4cfac9a40e3db0ee2fb

Now, a stable variable may hold the mutable state, and its partner, a flexible variable that shares the stable state, wraps it with the necessary higher-order functions. The entire API is available to the code that defines a pair of variables this way, just like in @nomeata's more OO version.

Compared with that version, the module-based version here is more cumbersome, as it uses no class at all, just a module and functions, forcing users to be very aware of which API calls require which kind of argument. However, if the two variables are initialized with sharing, there is no inherent lack of functionality here.

matthewhammer avatar Nov 09 '21 22:11 matthewhammer