planetkit icon indicating copy to clipboard operation
planetkit copied to clipboard

Split up and rename `CellPos` and friends

Open jeffparsons opened this issue 8 years ago • 1 comments

I'm planning a bit of a clean-up/refactor of what is currently in the globe module. The first part will be to move everything that relates purely to our Discrete Global Grid out into a separate top-level grid module, and then split up and rename a few of the structures. The second step will be a similar clean-up of everything to do with the voxmap, chunks, etc.

For clarity, what's in for this first step:

  • CellPos
  • Dir
  • PosInOwningRoot
  • Root
  • (RootResolution, which doesn't yet exist, but I'll use as a synonym for [IntCoord; 2])
  • Any functions that deal with these types.

The refactor bit will be to split CellPos into 2-dimenstional and 3-dimensional variants, because often I only deal with a "column" on the grid rather than a point in 3D grid space. I dismissed this earlier ("why not just ignore the z-axis?"), but I'm increasingly finding that I'd like the clarity about that encoded at the type level.

So... here comes the hard bit that I'd like input on: naming. I'm currently leaning towards (in roughly the same order as above):

  • GridPt2
  • GridPt3
  • Dir
  • (TODO: think about PosInOwningRoot; that can follow from whatever we decide for the rest)
  • Root
  • RootResolution

But I have a few questions in mind that might push me toward different naming:

  • Should I avoid abbreviations, like Nalgebra now does with its Point2 etc.? That would mean names like GridPoint2.
  • Should I just call it Point2, and rely on name-spacing to disambiguate it from the other Point2 (real coordinates, from Nalgebra) used in PlanetKit? That sounds like a recipe for confusion to me.
  • If I keep the Grid prefix on GridPoint2, should I add it to everything (e.g. GridDir) for consistency? Or would that be Emerson's "foolish consistency"? :sweat_smile:
  • Should I not qualify RootResolution, and just call it Resolution instead? It's only qualified now because there's a type in the globe module for chunk resolution, but maybe only that (less frequently used) type really needs its name qualified like that.

Any and all thoughts on these questions would be much appreciated! :smile: :heart:

jeffparsons avatar May 11 '17 16:05 jeffparsons

So far I have done these:

  • Rename CellPos to GridPoint3.
  • Split out GridPoint2.

I haven't yet cleaned up any of the other names, or introduced any of the other types suggested above.

jeffparsons avatar Jun 06 '17 17:06 jeffparsons