HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Optimizing smoosh() method further

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
furthersmooshoptimizingmethod

Problem

Part I of this question says:


Fill in the smoosh() method in the Homework3 class so that it performs as indicated in the comment. Your solution should not use linked lists, nor should it use your squish() method.

Here is the below code written for smoosh(), in more than 14 lines. Modular testing is done for this method smoosh().

```
public class Homework3 {

/**
* smoosh() takes an array of ints. On completion the array contains
* the same numbers, but wherever the array had two or more consecutive
* duplicate numbers, they are replaced by one copy of the number. Hence,
* after smoosh() is done, no two consecutive numbers in the array are the
* same.
*
* Any unused elements at the end of the array are set to -1.
*
* For example, if the input array is [ 0 0 0 0 1 1 0 0 0 3 3 3 1 1 0 ],
* it reads [ 0 1 0 3 1 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 ] after smoosh()
* completes.
*
* @param ints the input array.
**/

public static void smoosh(int[] a) {
// Fill in your solution here. (Ours is fourteen lines long, not counting
// blank lines or lines already present in this file.)
int currentPointer = 1;
int i, j = 0;
for(i =0; i < a.length; i++){
int flag = 0;
for(j = currentPointer; j < a.length; j++)
if(a[j] != a[i])
{
a[i+1] = a[j];
currentPointer = ++j;
flag = 1;
break;
}
if(j == a.length){
if(flag == 1)
i+=2;
else
i += 1;
for(int k = i; k < a.length; k++)
a[k] = -1;
break;
}
}
}

/**
* stringInts() converts an array of ints to a String.
* @return a String representation of the array.
**/

private static String stringInts(int[] ints) {
String s = "[ ";
for (int i = 0; i < ints.length; i++) {
s =

Solution

Your code advances an index until it finds an element that is different
from the current element a[i] and copies that to a[i+1]. The "filling"
part is bit involved because you have to distinguish whether an element
was copied into a[i+1] or not (your flag).

To work towards the given line limit, you use "non-trivial" for and if
statements without braces { ... }, which does not increase the
legibility and is error-prone (if more statements are added to
the for/if-expression later).

It becomes simpler if you think the other way around: Copy the current
element from the original position to the target position first, then
advance the original position until a different element is found.
This will copy some elements unnecessarily to itself, but leads to the following
function (13 lines in total without the comments) which should be self-explaining:

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


(Everything apart from the smoosh() function itself seems to be prescribed
by the given homework skeleton, so I am not going to comment on that.)

Code Snippets

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

Context

StackExchange Code Review Q#67972, answer score: 6

Revisions (0)

No revisions yet.