Skip to content

Refactor `Index`, etc. into `Select`, `Range`, and `Index`

Michael Aaron Murphy requested to merge huntergoldstein:index_refactor into master

Created by: huntergoldstein

Problem: The extra features introduced in #297 made it clear that Index IndexStart and IndexEnd were confusing to exist all together.

Solution: This PR consolidates and appropriately separates the responisbilities of Index, IndexStart, and IndexEnd into Select, Range, and `Index.

Changes introduced by this pull request:

  • Replaced top-level Index with Select which better captures the meaning of the enumeration: a selection of an array versus an index, which brings to mind a single element within the array.
  • Added a separate Range struct that encapsulates inclusive vs. exclusive
  • Consolidated Index<blah> into Index::Forward and Index::Backward and removed all of the CatchAll values. CatchAll is now represented by Forward(0) or Backward(0).
  • Added explicit, versus implicit, failure cases when a range is invalid

Drawbacks:

  • Selection is a more apt name but I wonder if there's something more precise
  • This is mostly due to the features in #297 but there are several places where some logic is repeated

TODOs:

  • Move some of the repeated logic and patterns for iterators from their call-sites to Range; I would love it if this PR reduced the total line count
  • Add documentation and real examples for Range and Index
  • Add in more regression tests, especially now that Range is easily independently testable

State: Good to go

Related: #297, #263 (closed)

Merge request reports