# Comparing two implementations of smoosh()

In this question, the suggested solution for smoosh() is as shown below,

public static void smoosh(int[] a) {
int originalPos = 0;
int targetPos = 0;
while (originalPos < a.length) {
// Copy (and remember) one element to the correct position:
int currentElement = a[targetPos++] = a[originalPos++];
// Advance originalPos until a different element is found:
while (originalPos < a.length && a[originalPos] == currentElement) {
originalPos++;
}
}
// Fill remaining elements:
while (targetPos < a.length) {
a[targetPos++] = -1;
}
}


but, I felt that this well tested new solution shown below looks more readable than shown above,

  public static void smoosh(int[] a) {
int saveOriginalPosition = 1;
int targetPosition = 0, originalPosition = 0;
while(targetPosition < a.length){
if(saveOriginalPosition < a.length){
for(originalPosition = saveOriginalPosition;originalPosition < a.length;originalPosition++)
if(a[originalPosition] != a[targetPosition])
{
a[++targetPosition] = a[originalPosition];
break;
}
saveOriginalPosition = originalPosition;
}else{
break;
}
}
if(originalPosition == a.length){
for(int setRemaining = targetPosition+1; setRemaining  < a.length; setRemaining++)
a[setRemaining] = -1;
}

}


because point mentioned by 200_success, i.e., "However, being able to recognize the structure of the loop just by looking at the loop header gives it an advantage in readability." is considered in this new solution, whereas the inner while loop of the suggested solution does not look readable.

• i did not prefer writing while((targetPosition < a.length) && (saveOriginalPosition < a.length)){ for the same reason and instead left a break with if{} block. Commented Oct 28, 2014 at 2:44

I think there are issues with both implementaitons, though I do find the first more readable.

The main problem is that neither makes it immediately clear that we are assigning a new value to each element of the input array. I would expect an implementation to look something like this:

public static void smoosh(int[] input) {
for (int i = 0; i < input.length; i++) {
input[i] = ...
}
}


This makes it clear to the reader that each element of the input array will be assigned to.

Now we can keep track of where the next element for our array is coming from:

public static void smoosh(int[] input) {
int j = 0;
for (int i = 0; i < input.length; i++) {
input[i] = j < input.length ? input[j] : -1;
j++;
while (j < input.length && input[i] == input[j]) {
j++;
}
}
}


It's getting hard to read again, so let's split the logic for j into its own method:

public static void smoosh(int[] input) {
int j = 0;
for (int i = 0; i < input.length; i++) {
input[i] = j < input.length ? input[j] : -1;
j = getNextDifferentIndex(input, j);
}
}

/** Get the smallest integer j such that j > i and input[i] != input[j].
* If no such element exists, return an integer greater than or equal to
* input.length. */
private static int getNextDifferentIndex(int[] input, int i) {
int j = i + 1;
while (j < input.length && input[i] == input[j]) {
j++;
}
return j;
}


Or if you want to stick to the 14-line limit mentioned in the previous question,

public static void smoosh(int[] input) {
for (int i = 0, j = 0; i < input.length; i++, j = getNextDifferentIndex(input, j)) {
input[i] = j < input.length ? input[j] : -1;
}
}

private static int getNextDifferentIndex(int[] input, int i) {
int j = i + 1;
while (j < input.length && input[i] == input[j]) {
j++;
}
return j;
}

• For your point:neither makes it immediately clear that we are assigning a new value to each element of the input array Do you mean: a[++targetPosition] = a[originalPosition]; does not convey this msg? Commented Oct 30, 2014 at 9:59