Page 1 of 1

## 11 Replies - 1331 Views - Last Post: 05 December 2012 - 09:52 PM

#### Is a Recursive Clear to read

1. Yes, when used correctly (7 votes [87.50%] - View)

Percentage of vote: 87.50%

2. No, it is never clear (0 votes [0.00%])

Percentage of vote: 0.00%

3. In between - only sometime clear (1 votes [12.50%] - View)

Percentage of vote: 12.50%

Vote Guests cannot vote

### #1 code_m

Reputation: 24
• Posts: 202
• Joined: 21-April 09

Posted 12 July 2011 - 04:38 PM

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.)

Is This A Good Question/Topic? 0

### #2 cfoley

• Cabbage

Reputation: 2243
• Posts: 4,722
• Joined: 11-December 07

Posted 13 July 2011 - 02:34 AM

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;
}

```

### #3 cfoley

• Cabbage

Reputation: 2243
• Posts: 4,722
• Joined: 11-December 07

Posted 13 July 2011 - 02:51 AM

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;
}
```

### #4 cfoley

• Cabbage

Reputation: 2243
• Posts: 4,722
• Joined: 11-December 07

Posted 13 July 2011 - 02:59 AM

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;
}
```

### #5 code_m

Reputation: 24
• Posts: 202
• Joined: 21-April 09

Posted 05 December 2012 - 07:23 AM

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.

### #6 lordofduct

• I'm a cheeseburger

Reputation: 2540
• Posts: 4,641
• Joined: 24-September 10

Posted 05 December 2012 - 07:46 AM

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.

### #7 code_m

Reputation: 24
• Posts: 202
• Joined: 21-April 09

Posted 05 December 2012 - 09:29 AM

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

### #8 jon.kiparsky

• Pancakes!

Reputation: 8933
• Posts: 15,439
• Joined: 19-March 11

Posted 05 December 2012 - 09:33 AM

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.

### #9 cfoley

• Cabbage

Reputation: 2243
• Posts: 4,722
• Joined: 11-December 07

Posted 05 December 2012 - 11:19 AM

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.

### #10 code_m

Reputation: 24
• Posts: 202
• Joined: 21-April 09

Posted 05 December 2012 - 12:21 PM

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

### #11 BobRodes

• Lovable Curmudgeon

Reputation: 592
• Posts: 3,047
• Joined: 19-May 09

Posted 05 December 2012 - 09:39 PM

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...

### #12 jon.kiparsky

• Pancakes!

Reputation: 8933
• Posts: 15,439
• Joined: 19-March 11