search

Fil / d3-geo-voronoi

Voronoi / Delaunay tessellations on the sphere
ISC License
229 stars 24 forks source link

geo_hull() Use more semantically meaningful Set and Map. #23

Closed martinfrances107 closed 3 years ago

martinfrances107 commented 3 years ago

I have been inspecting this code line by line while I port it over to rust.

This issue is very minor I want to refactor some code that uses

object 'key' => 'value' properties

But I think better structures such as Map and Set should be employed.

let me explain something minor that gives me unease.

  if (_hull[code]) delete _hull[code];
  else _hull[e.join("-")] = true;
}

for example after this loop _hull object will look something like

{ '4-3': true, '3-1': true, '0-4': true, '1-0': true }

A) Given the 'value' in the key: value pair is always a 'true' ... I suggest using a set Set structure so the machine does not need to repeatedly store the value.

B) While this code typical java-script and bug free.. the possible values passes into the if condition are TRUE when the key-value pair is found and NULL/UNDEFINED otherwise ..but if we used the Set method has() then the condition is restricted to just TRUE or FALSE.

Secondly _index is a Map like data structure which is currently a Object.

Anyway I have a patch ...l which I am going to link to this issue.

Fil commented 3 years ago

excellent! thank you

Fil commented 3 years ago

While working on this code this morning, I realized that it is in some places almost unreadable, and has poor test coverage. Using more semantic methods can also help with that.