Problem
After I couldn’t find any matching algorithm that would Interlace arrays I wrote this generic static function to combine arrays with variable segment sizes with n number of arrays.
The theory of this algorithm is that one would specify the indices in order of the params arrays with a number to indicate the segment lengths. So for example:
indices = {2,3}
arrayA = {a,b,c,d,e,f}
arrayB = {1,2,3,4,5,6,7,8,9}
result = {a,b,1,2,3, c,d,4,5,6, e,f,7,8,9}
The reason behind this Interlace algorithm is so that I could create vertex buffer objects friendly for opengl where segments of an array can mean different things, for example first three values of an element are x,y,z and the second three values are r,g,b.
public static T[] Interlace<T>(uint[] indices, params T[][] arrays) {
if (arrays.Length == 0) throw new ArgumentException("No arrays");
if (arrays.Length == 1) return arrays[0];
if(indices.Length != arrays.Length) throw new ArgumentException("Index count is not same as array count");
uint componentSize = 0;
foreach (uint i in indices) {
componentSize += i;
}
uint elements = (uint)arrays[0].Length / indices[0];
for (int i = 1; i < arrays.Length; ++i) {
uint numElements = (uint)arrays[i].Length / indices[i];
if (numElements != elements)
throw new ArgumentException("Specified arrays are not consistent sizes");
elements = numElements;
}
T[] result = new T[elements * componentSize];
for (uint i = 0; i < elements; ++i) {
uint offset = 0;
for (uint j = 0; j < indices.Length; ++j) {
offset += j == 0 ? (i * componentSize) : indices[j-1];
for (uint k = 0; k < indices[j]; ++k) {
result[k + offset] = arrays[j][k + (i * indices[j])];
}
}
}
return result;
}
Does this algoryithm seam over-complicated to achieve this goal?
Have I oversighted a simpler and more obvious approach?
Is there a better name I could use for the indices parameter?
Solution
Note that your code for checking that the inputs are ‘consistent’ involves rounding, so you might ignore the last few elements, and could obscure an insidious bug (e.g. off-by-one) somewhere else:
uint numElements = (uint)arrays[i].Length / indices[i];
if (numElements != elements)
Checking arrays[i].Length % indices[i] == 0 is probably how I’d solve this. Such checks would also be incompatible with the behaviour of if (arrays.Length == 1) return arrays[0];.
elements = numElements; is redundant and I think will only create confusion. I’d consider renaming numElements to something which implies its provisionalness.
As t3chb0t has said, the message in "Specified arrays are not consistent sizes" could be more useful. A dedicated check that indices[i] is non-zero might be appreciated, since it will manifest as a division by zero at the moment (granted it could be worse). You probably ought to throw in a null check as well, so that when the consumer makes a mistake he immediately knows what was null and doesn’t spend 30 seconds moaning about the API while they work it for themselves.
I’d expect the code to check (indices.Length != arrays.Length) before fast-pathing if (arrays.Length == 1) return arrays[0];, and that fast-path really needs documenting if it is an actual use-case, since it isn’t returning a copy. Inline documentation (///) takes little time to add, and really helps.
The nested loops at the end are rather confusing, especially offset, which I’d replace with a single incrementing position counter which is trivial to understand, and makes plain the ‘progressive’ nature of the loops.
uint pos = 0; // current position in result
for (uint i = 0; i < elements; ++i) {
for (int j = 0; j < indices.Length; ++j) {
for (uint k = 0; k < indices[j]; ++k) {
result[pos] = arrays[j][(i * indices[j]) + k];
pos++;
}
}
}
(not tested)
I agree that indices is not a good name; elementLengths might be better, since it holds the lengths of each of the elements of a component. "Index count is not same as array count" should also be changed in accordance.
I’m guessing you have a good reason to make it uint, but generally life is easier if int is used everywhere if you do not. Nothing in your code would stop working if you were to switch to int, but it would of course change the logical API.
You have a missing space after the third if.