Skip to content
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

Order is lost by match's object #2

Closed
mjgpy3 opened this issue Dec 29, 2015 · 6 comments · Fixed by #6
Closed

Order is lost by match's object #2

mjgpy3 opened this issue Dec 29, 2015 · 6 comments · Fixed by #6

Comments

@mjgpy3
Copy link
Contributor

mjgpy3 commented Dec 29, 2015

I'm not sure if this is desired or not, but the following test

  describe('when multiple values can be hit', () => {
    it("doesn't hit the first one",  () => {
      t.strictEqual(match({
        [when.range(0, 43)]: 42,
        [when(42)]: 72,
        [when()]: 'never should be hit',
      })(42), 42);
    });
  });

fails with

AssertionError: expected 72 to equal 42

Not sure if it's really an issue or not, thoughts?

@mjgpy3
Copy link
Contributor Author

mjgpy3 commented Dec 29, 2015

I'm bringing it up because:

  1. Those familiar with pattern matching expect a top to bottom order
  2. If a user accidentally has overlapping patterns (as in the case of above) the decision to pick one seems arbitrary (where it's hashed into the object)
  3. This will make implementing wildcard matches difficult because they tend to have multiple possible matches

@igl
Copy link

igl commented Dec 29, 2015

JS objects are an unordered collection of properties.
This could be fixed by using a Map (ES6)

@igl
Copy link

igl commented Dec 29, 2015

describe('when multiple values can be hit', () => {
  it("doesn't hit the first one",  () => {
    t.strictEqual(match(new Map([
      [ when.range(0, 43), 42 ],
      [ when(42), 72 ],
      [ when(), 'never should be hit' ]
    ]))(42), 42);
  });
});

Not that pretty though... and one could use regular arrays in the first place. Meh TC39.

@mjgpy3
Copy link
Contributor Author

mjgpy3 commented Dec 30, 2015

Another possible syntax is fluent chaining

  when.range(0, 43, 42).
  when(42, 72).
  otherwise('will not be hit').
  match(42)

@eranimo
Copy link

eranimo commented Dec 30, 2015

Would it be possible to support both chaining and objects?

@FGRibreau
Copy link
Owner

Hello @mjgpy3 @eranimo @igl,

I knew this property of JS object but did not took the time to look into this since my main goal was first to make it work :)

Now I've looked into this and found a solution to make it work (generating an incremental unique id at each call of when() and since when() will always be in the same callsite as the object creation it will work everytime.

I'm really against fluent chaining and only use it when I can't do it another way.

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

Successfully merging a pull request may close this issue.

4 participants