Follow-up from "Pixel Tracking with Phi-sorted hits and Phi-search windows, Include Flags for…"

The following discussions from !926 (closed) should be addressed:

@graven started a discussion:
If you write this as:
```
#include "Kernel/STLExtensions.h"

void fill( LHCb::span<const LHCb::VPLightCluster> clusters,
           LHCb::span<const size_t,35> hitbuffer) 
```
with no changes to the implementation, then you will be able at some point to change the storage of clusters and hitbuffer (as long as they remain contiguous, which is a good bet) without having to change this function.

why not write this as a switch statement:

  switch(configuration) {
  case Conf::Forward:
  case Conf::Default:  next -= value; break;
  case Conf::Backward: next += value; break;
  }

@graven started a discussion: (+1 comment)

please avoid the preprocessor if you can -- and in this case, you can:

constexpr auto ONEQTR_PI = M_PI/4.0;
constexpr auto THRQTR_PI = 3.0*M_PI/4.0; // note: auto to be consistent with what you had -- use double/float instead if that matters
constexpr float RADDEG = 180.0 / M_PI;
constexpr float PI_FLOAT = M_PI;
constexpr float PIBY2_FLOAT = M_PI/2;

@graven started a discussion:
please use
```
#include <cmath>
```
instead
@cattanem started a discussion:

I don't really understand why you have to provide your own version of this. Surely it already exists in vdt or similar? And even if it doesn't, it should be put in LHCb/Kernel/LHCbMath where others can benefit from it, not buried here.
@graven started a discussion:

Note that std::stable_sort is slower than std::sort (*). So if the results depend on the difference between stable_sort and sort, then typically it is better to make the ordering criteria to be more strict -- i.e. in case it currently classifies its argument as equal, explicitly define the order of those elements (specifically, guarantee that (a<b) == !(b<a) always holds -- ie. if you can tell the difference between stable_sort and sort, then by construction the values must have some attribute that distinguishes what the current comparator ignores -- so use that attribute as a 'tie breaker' in case the current comparator considers elements equal)

(*) compare the complexity statements for stable_sort -- O( N log^2 (N) if no memory available, or O(N log(N)) if there is memory, i.e. it either allocates memory, or will be forced to use a slower algorithm -- vs. sort which is plain O(N log N) without needing memory (and presumably has a smaller constant of proportionality)

Edited Aug 06, 2018 by Sascha Stahl