10
\$\begingroup\$

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?

\$\endgroup\$
5
  • \$\begingroup\$ Consider where you can use standard algorithms. For example componentSize is just an array sum: does this work? stackoverflow.com/a/2419351/1688786 \$\endgroup\$ – Josiah Oct 23 '18 at 9:26
  • 2
    \$\begingroup\$ I didnt use Sum as this is a Linq function. I tend to avoid linq as much as possible as this function may be ported to c/c++ in the future \$\endgroup\$ – Gelion Oct 23 '18 at 9:29
  • 2
    \$\begingroup\$ I find this message a little bit confusing and not helpful: "Specified arrays are not consistent sizes" - don't say just what is wrong, tell the user what is correct. E.g. all arrays must have following sizes... because otherwise they will wonder what consistent sizes mean - like I do and without debugging it or seeing the source code, it would be impossible to figure this out. \$\endgroup\$ – t3chb0t Oct 23 '18 at 9:42
  • 4
    \$\begingroup\$ A big vote for sharing the actual purpose of this method. Knowing it's for opengl is a huge help because it virtually automatically excludes linq ;-) \$\endgroup\$ – t3chb0t Oct 23 '18 at 9:44
  • 2
    \$\begingroup\$ If performance is important to you, then you may want to experiment with the order of those loops. I suspect that iterating the arrays/indices in the outer loop will result in less cache misses. \$\endgroup\$ – Pieter Witvoet Oct 23 '18 at 11:25
11
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.