Refactor ExecutionSegment sequences, wrappers and tuples #1446
Conversation
|
hmm not sure why this linter issue doesn't show locally 😕 I'll force-push... |
This cleans up the interfaces and makes it possible to re-use the heavy calculations for a segment sequence.
Codecov Report
@@ Coverage Diff @@
## new-schedulers #1446 +/- ##
==================================================
- Coverage 76.34% 76.31% -0.03%
==================================================
Files 162 162
Lines 12936 12937 +1
==================================================
- Hits 9876 9873 -3
- Misses 2545 2550 +5
+ Partials 515 514 -1
Continue to review full report at Codecov.
|
| // NewExecutionTuple returns a new ExecutionTuple for the provided segment and | ||
| // sequence. | ||
| // | ||
| // TODO: don't return a pointer? |
There was a problem hiding this comment.
The Pointer was required because fillCached needed to change the tuple so ... now this can be a not pointer :D
There was a problem hiding this comment.
yeah, and I started doing it, but then I had to change like a 1000 uses... I don't mind doing it now, but if I had done it, I imagined you would have said (not incorrectly) something like "this could have been done later" 😉
There was a problem hiding this comment.
It probably would - I remember moving from the non-pointer to the pointer variant and it took half an afternoon :). To be honest I don't think this is worth an issue, and it's likely won't better anything if changed at this point. I just wanted to point (pun intented :P) out why this was the case :D
| prev[index] = iteration | ||
| if int64(len(offsets[index])) == numerator { | ||
| offsets[index] = append(offsets[index], offsets[index][0]+lcd-iteration) | ||
| } |
There was a problem hiding this comment.
I like that this lambda is not external anymore, and the algorithm is in a single place now. 👍
This cleans up the interfaces and makes it possible to re-use the heavy calculations for a segment sequence.
closes #1379