G-Node / nix

Neuroscience information exchange format
https://readthedocs.org/projects/nixio/
Other
66 stars 36 forks source link

adaptations for RangeDimension indexOf methods #819

Closed jgrewe closed 4 years ago

jgrewe commented 4 years ago

This pull request is the first step for fixing the issue raised in #818 Adds some overloads to the RangeDimension::indexOf method family that allow for some behavior controlling. Unfortunately, the original indexOf behavior needed to be altered a bit. Invalid positions will not raise an exception. There is, however a new method that allows for cheking if a position is in range.

I made some decisions I would like to discuss:

  1. is it a good idea to allow end be less than start? At the moment I check and simply swap the variables (line 467 in Dimension.cpp).
  2. is it a good idea to silently ignore invalid ranges? Currently they are simply not part of the result vector. Alternatively the vector could contain optionals.
  3. The method for getting start and end indices of a single range (start and end positions) now also takes a vector of ticks (lines 460 et seq.). This to avoid mulitple retrievals of the ticks which would happen because this method might be repeatedly called by the vector-of-starts-and-ends version. Issue is, that I want to have the ticks argument optional. , i.e. method calls with {} instead of a vector. This forbids to pass the ticks vector as reference. So there is some copying going on, is there a good way to avoid this?

edit: test failures are expected :)

edit 2: I think this thing is good for review. Regarding the questions stressed above, the current states are:

  1. yes, I think so and this done exactly this way
  2. no, it is not. There are overloaded functions that return optionals which are not initialised for invalid ranges. Original functions (deprecated) will throw errors.
  3. kept it like that, tried in an intermediate version an rValue reference but, (according to @gicmo) repeated std::move is not a good idea, so this was removed again