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

Construction and addition order #32

Open
Ginden opened this Issue Feb 5, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@Ginden
Copy link
Collaborator

Ginden commented Feb 5, 2019

.filter and similar methods can either:

  1. Create new instance, perform algorithm on internal data structure, and then use AddEntriesFromIterable.
  2. Perform algorithm on internal data structure, and then create new instance (it will internally use AddEntriesFromIterable)

This can be observed by user in few twisted cases. Eg.

class FooSet extends Set {
  add(...args) { 
      console.log('foo');
      return super.add(...args);
   }
}

(new FooSet([1])).filter(() => console.log('bar'));

This code, depending on our choice, will either log foo,bar or bar,foo.

Personally I'm in favor of first option.

@ljharb

This comment was marked as off-topic.

Copy link
Member

ljharb commented Feb 5, 2019

Because the constructor calls add, it would have to log foo,foo,bar or foo,bar,foo

@zloirock

This comment was marked as off-topic.

Copy link
Contributor

zloirock commented Feb 6, 2019

@ljharb console.log returns undefined -> .add will be called once on new FooSet([1]), but not once on result set. Why do you expect 2 foo?

@ljharb

This comment was marked as off-topic.

Copy link
Member

ljharb commented Feb 6, 2019

Because filter calls add by constructing a new set? But you’re right that bar wouldn’t be added anywhere because the predicate returns undefined

@zloirock

This comment was marked as off-topic.

Copy link
Contributor

zloirock commented Feb 6, 2019

Empty set.

@zloirock

This comment was marked as off-topic.

Copy link
Contributor

zloirock commented Feb 6, 2019

For

class FooSet extends Set {
  add(...args) { 
      console.log('foo');
      return super.add(...args);
   }
}

(new FooSet([1])).filter(() => console.log('bar') || true);

should be foo,bar,foo.

@zloirock

This comment has been minimized.

Copy link
Contributor

zloirock commented Feb 6, 2019

More interesting this case:

class FooSet extends Set {
  add(...args) { 
      console.log('foo');
      return super.add(...args);
   }
}

(new FooSet([1, 2])).filter(() => console.log('bar') || true);

With AddEntriesFromIterable we will have foo,foo,bar,bar,foo,foo. But for me seems more logical foo,foo,bar,foo,bar,foo and direct adding of value after each calling of callback.

@Ginden

This comment has been minimized.

Copy link
Collaborator Author

Ginden commented Feb 11, 2019

Working example:

class FooSet extends Set {
   constructor(...args) {
      console.log('ctr');
      super(...args);
   }
   add(val) {
       console.log('add');
       return super.add(val);
    }
}

(new FooSet([1, 2])).filter(() => {
   console.log('predicate');
   return true;
});

This will always print ctr => add => add first (because of (new FooSet([1, 2]))).

Then it can print one of these:

  1. ctr => predicate => add => predicate => add ("create set and add element after each call to predicate")
  2. predicate => predicate => ctr => add => add ("use internal list, then create set from that list")
  3. ctr => predicate => predicate => add => add ("create set, operate on internal list, then add elements to set")

Array use approach 1, but it isn't observable to user unless Proxy with defineProperty trap is returned from FooArray subclass (gist). Therefore, I'm in favor of 1.

@zloirock @ljrharb Can I hide your comments to make thread clearer to readers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment