[code_m]

At work I needed to come up with a clear way (in syntax) to take a given byte array, look for a given byte value and add it with the next value in the array, then remove that value to the right from the array.

I came up with a recursive function that I think is clear:

private static byte[] JoinByteValues(byte[] message, byte valueToJoin, int start)
{
    int joinIndex = Array.IndexOf<byte>(message, valueToJoin, start);

    if (joinIndex > -1 && joinIndex < message.Length)
    {
        // copy to here, minus one byte
        byte[] shorten = new byte[message.Length - 1];

        // lets put the values back together at the index of joinIndex
        message[joinIndex] = (byte)(message[joinIndex] + message[joinIndex + 1]);

        // copy message to shorten, skipping the index [joinIndex + 1]
        Array.Copy(message, 0, shorten, 0, joinIndex + 1);
        Array.Copy(message, joinIndex + 2, shorten, joinIndex + 1, message.Length - joinIndex - 2);

        // recall this method with the shorten copy, to find any more joinIndex values
        return JoinBytesValue(shorten, valueToJoin, joinIndex + 1);
    }
    else
    {
        // no more bytes need joined
        return message;
    }
}

Today I also came up with this function without recursion, but I don't think it's any clearer:

private static void JoinByteValues(ref byte[] message, byte valueToJoin, int start)
{
    int joinCount = 0;

    for (int joinIndex = Array.IndexOf<byte>(message, valueToJoin, start);
             joinIndex > -1 && joinIndex < message.Length;
             joinIndex = Array.IndexOf<byte>(message, valueToJoin, start))
    {
        // add values back to together and store it at joinIndex
        message[joinIndex] = (byte)(message[joinIndex] + message[joinIndex + 1]);

        // start is the value we are 'dropping' out of the array
        start = joinIndex + 1;

        // move everything to the right of start left one index
        Array.Copy(message, start + 1, message, start, message.Length - start - ++joinCount);
    }

    if (joinCount > 0)
    {
        // Drop the empty indexes do the the movement of data
        Array.Resize<byte>(ref message, message.Length - joinCount);
    }
}

So I'm curious as to which way you would rather see it? Both functions pass my tests. (Also, speed is not a concern, I am working with a small data set, never more than 90 bytes. I am only interested to know which you prefer readability wise.)

[cfoley]

Recursive or iterative, that code could be clarified by splitting it up into smaller methods. Please excuse the Java:

	byte[] joinByteValues(byte[] arr, byte value, int start) {
		int index = find(arr, value, start);
		// It's length-1 because if we find the value in the last element
		// there is no following element to add it to.
		if (index >= arr.length - 1) return arr;
		
		byte[] chopped = removeIndex(arr, index);
		chopped[index] += value;
		return joinByteValues(chopped, value, index);
	}

	// Returns the index of the value beginning with start
	// Or returns arr.length (i.e. an out of bounds index)
	int find(byte[] arr, byte value, int start) {
		for(int i = start; i < arr.length; i++) {
			if (arr[i] == value) return i;
		}
		return arr.length;
	}
	
	byte[] removeIndex(byte[] arr, int index) {
		byte[] ret = Arrays.copyOf(arr, arr.length-1);
		for(int i = index;i < ret.length; i++) {
			ret[i] = arr[i+1];
		}
		return ret;
	}

[cfoley]

Or my attempt at refactoring your code:

private static byte[] JoinByteValues(byte[] arr, byte value, int start) {
    int index = Array.IndexOf<byte>(arr, value, start);
    if (index > -1 && index < arr.Length){
        byte[] chopped = removeIndex(arr, index);
        chopped[index] += value;
        return JoinBytesValue(chopped, value, index + 1);
    } else {
        return arr;
    }
}

private static byte[] removeIndex(byte[] arr, int index) {
	byte[] ret = new byte[arr.Length - 1];
	Array.Copy(arr, 0, ret, 0, index);
	Array.Copy(arr, index + 1, ret, index, ret.Length - index);
	return ret;
}

[cfoley]

Incidentally, shouldn't this

return JoinBytesValue(chopped, value, index + 1);

be index, not index+1? What if the value you find is followed by a zero? Don't you want to remove that again and add it to the next one? For readability, it could now become:

private static byte[] JoinByteValues(byte[] arr, byte value) {
    int index = Array.IndexOf<byte>(arr, value);
    bool isFound = index > -1 && index < arr.Length;
    if (isFound){
        byte[] chopped = removeIndex(arr, index);
        chopped[index] += value;
        return JoinBytesValue(chopped, value);
    } else {
        return arr;
    }
}

private static byte[] removeIndex(byte[] arr, int i) {
	byte[] ret = new byte[arr.Length - 1];
	Array.Copy(arr,   0, ret, 0, i);
	Array.Copy(arr, i+1, ret, i, ret.Length - i);
	return ret;
}

[code_m]

cfoley, on 13 July 2011 - 04:59 AM, said:

be index, not index+1? What if the value you find is followed by a zero?

The function is to un-escape a received byte message. That's the why the protocol works.

Sorry for digging up an old post Anyway, I went with the recursive definition. In the end it didn't matter much, only one other developer even ever saw the code.

[lordofduct]

Wouldn't choice 1 and 3 of your poll be nearly the same?

I could say that it's only sometimes clear. When you're doing it clearly, it's correct, and when it's not clear, you're doing it incorrectly.

[code_m]

Perhaps. Important thing is that no one chose "2".

[jon.kiparsky]

Quote

Perhaps. Important thing is that no one chose "2".

Well, of course. That's a straw man.

I like the auto-necro by the way. Usually it's someone else who raises a topic from the dead a year later.

[cfoley]

code_m, on 05 December 2012 - 02:23 PM, said:

cfoley, on 13 July 2011 - 04:59 AM, said:

be index, not index+1? What if the value you find is followed by a zero?

The function is to un-escape a received byte message. That's the why the protocol works.

Sorry for digging up an old post /> Anyway, I went with the recursive definition. In the end it didn't matter much, only one other developer even ever saw the code.

Then I guess your code can't have been clear enough for me.

[code_m]

Don't yank my chain too hard, I need that neck!

[BobRodes]

jon.kiparsky, on 05 December 2012 - 10:33 AM, said:

Quote

Perhaps. Important thing is that no one chose "2".

Well, of course. That's a straw man.

I like the auto-necro by the way. Usually it's someone else who raises a topic from the dead a year later.

A subtle form of recursion perhaps...

[jon.kiparsky]

I always favored the definition of recursion that I got from my first computer teacher, in high school. "What's recursion? Well, you know what it means to curse, right? So clearly, to recurse you simply do that over and over."