Skip to content

Iterating on the design #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
codehag opened this issue Mar 26, 2020 · 7 comments
Closed

Iterating on the design #20

codehag opened this issue Mar 26, 2020 · 7 comments

Comments

@codehag
Copy link

codehag commented Mar 26, 2020

We reviewed this proposal back when it was presented, but I didn't manage to get our notes up.

One of the major comments was that the api is unintuitive as it tries to do too many things. There might be a better way to achieve the desired effect. A number of folks mentioned defaultdict as being the thing that they miss when writing js. The most common usage is the following: if (!map.has(k)) { m.set(k, []) }; m.get(k).push(val);

A suggestion from @jorendorff was new Map ( [iterable [, valueFn]] ). When the user calls map.get(key) with no second argument, and no entry for key exists, and valueFn was specified, it’ll call valueFn(key) and insert and return the resulting value. Same for WeakMap.

@ljharb
Copy link
Member

ljharb commented Mar 26, 2020

That sounds workable to me (altho i'd want the valueFn to be in an options bag, so more "internal hooks" could be added in the future).

@hax
Copy link
Member

hax commented May 1, 2020

Maybe we could simply overload map.get(k) with map.get(k, valueFn)?

@jorendorff
Copy link

An options bag sounds good.

@bmeck
Copy link
Member

bmeck commented Jul 1, 2020

I think the function being internal state to the collection might cause some usability issues as people have existing code that does the following:

let value = map.get(value) || generateDefault();
// mutate /  replace value
map.set(key, value);

Having .get do a defaulting operation here would mean it would suddenly not take that branch (could be seen as intentional arguably but I am not convinced at a glance that is good) and that multiple places using different values would be unable to give different defaults. You can imagine scenarios like the following as well:

function init(k) {
  return map.get(k); // should synchronously introduce it if it doesn't exist
}

function result(k) {
  return map.get(k); // should create an error/tdz
}

Other possible things are stuff like inserting synchronous values if the value would be known at the time of the .get vs asynchronous values if the value is not available and might need to be fetched somehow.

Additionally, one of the big usages of mine is to update existing values. With Record/Tuple/primitives/frozen values/etc. this does lean towards needing an update function. I think having a defaulted value is useful, but I do not think it would be the best path at this time unless we can convince ourselves that these state-less/read-only values are not needing to be updated in the common case. In my experience, if you get a default value you often want to perform some kind of update operation on it afterwards anyway.

@bmeck
Copy link
Member

bmeck commented Jul 9, 2020

I've moved feedback from several issues to a revised design in #21

@bergus
Copy link

bergus commented Feb 24, 2021

The most common usage is the following: if (!map.has(k)) { m.set(k, []) }; m.get(k).push(val);

Don't forget about counters (or any other updates with immutable values, where you can't just push to the array):

map.set(k, 1 + (map.has(k) ? map.get(k) : 0))
map.set(k, map.has(k) ? map.get(k) + 1 : 1))

This use case is not covered by default values. map.set(k, map.get(k, 0)+1) is closer but still doesn't get us all the way to a single (hash/tree) lookup.

@dminor
Copy link
Collaborator

dminor commented Oct 22, 2024

The new design direction is focusing on the get/insert if missing usecase, with update being left for a future proposal.

@dminor dminor closed this as completed Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants