Ethan
Ethan

Reputation: 25

Need help fixing my selection sorting

My method is supposed to return the position of the largest element in the unsorted portion of the array, the second parameter to the method specifies the largest index that is where the unsorted portion of the array ends. My testMaxPositionMiddle passes but my testMaxPositionFirst fails i don't understand why.

public Object maxPosition(int[] array, int x)
{

for( int i = 0; i < array.length - 1; i++)
{
    int maxPosition = i;
    for( int j = 0; j < array.length; j++)
    {
        if(array[j] > array[maxPosition])
        {
            maxPosition = j;
            return maxPosition;
        }
    }
}
return null;
}

@Test
public void testMaxPositionMiddle()
{
int[] x = { 1, 5, 3, 7, 8, 9};
MySorter s = new MySorter();
assertEquals(1, s.maxPosition(x,2));

}

@Test
public void testMaxPositionFirst()
{
int[] x = {5, 1, 3, 2, 8, 9};
MySorter s = new MySorter();
assertEquals(0, s.maxPosition(x, 3));

}

Upvotes: 0

Views: 63

Answers (1)

Sara S
Sara S

Reputation: 679

You have three errors in your code.

The first one is that you are actually checking for maxvalue in the entire array instead of checking for values between 0.....x. Therefore you should change your loop to not loop through entire array, but only the beginning:

for(int i = 0; i <= x; i++){

}

Your second problem is that you are running through 2 loops when you need only one: the one checking for the biggest value. You start by assuming that the biggest value is the value in the first (0) position, and then update maxvalue everytime you find a bigger one. Start by giving maxPosition the value 0, as a start value before the loop.

int maxPosition = 0;

Your third problem is that if the first position (0) actually contained the correct value, then maxPosition should be 0.

if(array[j] > array[maxPosition]){
    maxPosition = j;
    return maxPosition;
}

In these lines, you will return the correct maxPosition if it is updated. In the case that the maxPosition is 0, it will never be updated, and then you will never reach this line:

return maxPosition;

Instead, you will get to the return statement in the end of the function, and instead return null like in this line:

return null;

Actually your code has even a fourth error. In this state, you are returning any value that is bigger than maxPosition in the beginning.

For example, if you had a list (2,4,6) then maxPosition would be 0 in the start. maxValue would be 2 but these lines of code:

if(array[j] > array[maxPosition]){
    maxPosition = j;
    return maxPosition;
}

will actually make you jump inside the if-statement even in the second case where the value array[1] > array[maxPosition] since 4>2. Your code would therefore return the maxPosition 1

What you want to do is to remove the return-statement from inside the if-statement and only have one return statement for your function. That should be in the end of your total function, after the for-loop

Upvotes: 1

Related Questions