-1

Hey so I have this homework assignment and I'm having issues with one of the methods. I would like hints and not actual answers/code.

So I have a class called HorseBarn that messes with an array of horses(horse being the type). My problem is I'm having troubles with the consolidate method.

What the array would look like before consolidate: a,b,c,d are horses

|a|null|b|null|c|d|

What the array would look like after consolidate:

|a|b|c|d|null|null|

So my logic would be to make a nested for loop. The first loop would search for a null value, once the first loop finds the null value, the second loop would look for a horse and then swap with it. Then the second loop would end and go back to the first loop. So here is what I have right now and it doesn't work(it just terminates). Is my logic wrong or is it my syntax that's causing the problems?

public void consolidate()
{
    int j = 0;
    for(int i = 0; i < spaces.length;i++)
    {
        if( spaces[i] == null)
        {
            for(j = i; j < spaces.length && spaces[j] == null; j++)
            {

            }
            spaces[i] = spaces[j];
            spaces[j] = null;
        }

    }
user3716335
  • 5
  • 2
  • 1
    Your code seems OK at a first glance, how can you tell it's not working? – Reut Sharabani Jun 06 '14 at 20:01
  • Why is the inner `for` loop empty? Are the statements after it intended to be in the body of the loop? – David Conrad Jun 06 '14 at 20:03
  • @DavidConrad , think of it a "next-non-null" setter (it looks for the next "non-null" and sets it there. :) – Reut Sharabani Jun 06 '14 at 20:03
  • @Totoro , but isn't that what the if statement is doing? – user3716335 Jun 06 '14 at 20:04
  • @DavidConrad, there is a problem of putting objects in the last cell of the array, but the logic of the empty loop is ok. it's clearer to just terminate it with no curly-braces. – Reut Sharabani Jun 06 '14 at 20:04
  • @ReutSharabani , I have a test file that tells me if the the method worked or not. – user3716335 Jun 06 '14 at 20:05
  • can you tell us what the array looks like before and after the method? do **you** know? It's a different level of familiarity with the code to say "it's not working", and "I know how it's not working" – Reut Sharabani Jun 06 '14 at 20:05
  • -- Barn 2 (before consolidate) -- [0]: Trigger, 1340lbs. [1]: null [2]: Silver, 1210lbs. [3]: null [4]: null [5]: Patches, 1350lbs. [6]: Duke, 1410lbs. -- Barn 2 (after consolidate) -- [0]: Trigger, 1340lbs. [1]: Silver, 1210lbs. [2]: Patches, 1350lbs. [3]: Duke, 1410lbs. [4]: null [5]: null [6]: null – user3716335 Jun 06 '14 at 20:08
  • That is what it should look like, the output just returns 7 after consolidate is called. – user3716335 Jun 06 '14 at 20:09
  • isn't that exactly what the code should do? – Reut Sharabani Jun 06 '14 at 20:10
  • what does your code do to the array? what does `consolidate`, that you've written, do to the array? – Reut Sharabani Jun 06 '14 at 20:11

2 Answers2

1

Well for starters, this should give an index out of bounds exception if the last non-null is found and there still are elements remaining:

ex: horses = | a | null | null | null |

for i = 1, since horses[1] -> horses[3] are empty, j first gets set to 1 then ends with j = 4 (because of the termination condition j < horses.length())

You would then try to swap horses[1] with horses[4], which throws the array index out of bounds

Kevin L
  • 1,066
  • 3
  • 12
  • 21
0

In the inner for loop, just find the position of next non null value and break it there. Then swap it with your null. A better time efficient code.

Mayank Agarwal
  • 376
  • 2
  • 9