a StableHashMap module.
#300
Conversation
|
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 :-) |
|
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. |
this is to demonstrate what I meant in dfinity#300 (comment) and dfinity#299 (comment), and how to introduce this without breaking changes (although it’s kinda ugly) What I would _want_ here is to introduce a second, more general, constructor for the given class, but Motoko does not allow me to do that easily. But I can hack around that by * Creating a new class, not public `HashMap_` with the constructor I want * In `HashMap`’s constructor, call the `HashMap_` constructor to create an inner object (`i`) * In `HashMap`, simply copy all the fields from the inner objects to the outer object. * A public module-level function (here `wrapS`) exposes the new constructor. With this (generic, ugly) trick I can suppor the idiom ``` stable var userS : HashMap.S <UserId,UserData> = newS(); let user : HashMap.HashMap<UserId,UserData> = HashMap.wrapS(10, Nat.eq, Nat.hash, userS) ``` without changing the API. But it is ugly, and the effect on documentation generation is probably bad as well. So maybe a better course of action would be to have a midly breaking change where we only have the “new” constructor, and people will have to fix their code by passing `HashMap.newS()` as a new fourth argument if they want their old behavior. Probably better than piling up hacks like this. In that case, simply rename `class HashMap_` to `HashMap`, remove `wrapS` and the old `class HashMap`.
There was a problem hiding this comment.
I think I’d prefer the idiom described in #299 (comment) over adding new modules or classes.
There was a problem hiding this comment.
Hm, I'm not sure I understand how I would use this API in practice. The bifurcation between types HashMap and HashMap_ would seem to make it impossible to perform relevant operations on an actual stable hash-map, which I can't even use get on? What would I actually do with a plain HashMap?
I also see the benefits of not adding these to If so, then this PR is here because
Of course, each |
I admit that the API for that type is limited. One could fill a
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.
Well, |
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 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:
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: e7a2787 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 |
ggreif
force-pushed
the
master
branch
2 times, most recently
from
d52aecd
to
08507fc
Compare
An outgrowth the the discussion in #299
A non-OO,
stableversion of our (currently OO)HashMapimplementation.