search

anvaka / ngraph.graph

Graph data structure in JavaScript
BSD 3-Clause "New" or "Revised" License
520 stars 66 forks source link

Added useMap option to graph #22

Closed FelixMo42 closed 4 years ago

FelixMo42 commented 4 years ago

Allow people to use maps instead of objects because

  1. they are faster (755 ops/sec ±1.45% vs 6,462 ops/sec ±3.79%)
  2. they allow you to use objects as keys.

I would say make it the default, but not all browsers support it. Maybe make it check to see it it's supported and automatically enable it.

I made sure it passed all the tests.

PS, I love ngraph. I just started using it and it's awsome.

FelixMo42 commented 4 years ago

@anvaka hi, could you have a look at this. Sorry, this is my first time contributing to something like this, I'm not sure what proper protocol is.

anvaka commented 4 years ago

Hi @FelixMo42

Thank you for your contribution! I want to take a deeper look into your change and will come back within couple days.

anvaka commented 4 years ago

Love your change. Merged it in. Before publishing a new version, I'm going to update ngraph.graph so that it uses Map by default (no need to pass an option). Will probably need to think a bit more about for .. of operator as it seem to be not supported in IE.

anvaka commented 4 years ago

Okay, updated package to use the Map, and removed bunch of old code. Your performance test jumped from 6K ops per second to 12K.

I still haven't published this as a new version. Want to pull from my repository and play with it to see if it works for you?

FelixMo42 commented 4 years ago

I gave it a run and it looks good to me!

anvaka commented 4 years ago

Thank you! Published as v19.0.0