BHoM / BHoM_Engine

Internal manipulation of the BHoM
GNU Lesser General Public License v3.0
26 stars 13 forks source link

Geometry_Engine: consider renaming current `GeometryHash()` methods to `ToHashArray()`; expose a new `GeometryHash()` method that uses `ToHashArray()` to return a single number #3253

Closed alelom closed 7 months ago

alelom commented 7 months ago

As highlighted by @pawelbaran in #3251 , typically users would need and expect a single hash string or number to be returned by the GeometryHash method.

The methods we currently are in reality producing a unique double array which can be useful for producing a hash signature, but it's impractical in its current form. However, they can be used to power an "aggregator" function that returns a single double, whose implementation is the subject of #3251. Then, the existing methods may be renamed to something like ToHashArray(), while the new aggregator method could be called GeometryHash().

Finally, the ToHashArray() methods could even be made private – I'm not sure there is a use case for them without an aggregator function, they feel like "backend" methods returning a non-user-friendly list of doubles.

Thoughts? @al-fisher @pawelbaran

pawelbaran commented 7 months ago

Yeah I had similar thoughts @alelom, but did not want to distract the core question raised in #3251. I cannot think of a good implementation of an array in a real life scenario, but I appreciate the fact that it is more unique than the aggregation, so not entirely sure if we would not want to keep it public at least.

But in general, I agree 👍

al-fisher commented 7 months ago

Yes - in agreement both too on naming. And if we can solve https://github.com/BHoM/BHoM_Engine/issues/3251 nicely and elegantly then making the newly named ToHashArray methods private also makes some sense.

alelom commented 7 months ago

Implemented in https://github.com/BHoM/BHoM_Engine/pull/3257, where the final method name was Query.HashArray().