1

I have a very weird problem, the problem only presents itself in case 3 & 4.

                for(Sector sector : sectoren)
    {
        switch(sector.getCode())
        {
            case 1:
                for(int i = 0; i<3; i++)
                {
                    for(int j=0;j<3; j++)
                    {
                        Gebiedskaart gbk = gebiedskaarten.get(r.nextInt(gebiedskaarten.size()));
                        vakken[i][j].setGbk(gbk);
                        gebiedskaarten.remove(gbk);
                    }
                vakken[2][2].setGbk(null);
                }
            case 2:
                for(int i = 0; i<3; i++)
                {
                    for(int j=4;j<7; j++)
                    {
                        Gebiedskaart gbk = gebiedskaarten.get(r.nextInt(gebiedskaarten.size()));
                        vakken[i][j].setGbk(gbk);
                        gebiedskaarten.remove(gbk);
                    }
                }
                vakken[2][4].setGbk(null);
            case 3:
                for(int i=4; i<7; i++)
                {
                    for(int j=0;j<3; j++)
                    {
                        System.out.println(gebiedskaarten.size());
                        Gebiedskaart gbk = gebiedskaarten.get(r.nextInt(gebiedskaarten.size()));
                        vakken[i][j].setGbk(gbk);
                        gebiedskaarten.remove(gbk);
                    }
                }
                vakken[4][2].setGbk(null);
           case 4:
                for(int i = 4; i<7; i++)
                {
                    for(int j=4;j<7; j++)
                    {
                        Gebiedskaart gbk = gebiedskaarten.get(r.nextInt(gebiedskaarten.size()));
                        vakken[i][j].setGbk(gbk);
                        //gebiedskaarten.remove(gbk); doet iets raar? moeten we nog naar kijken, hij blijft gewoon verwijderen tot de lijst leeg is
                    }
                }
                vakken[4][4].setGbk(null);
        }
    }

in the code you can see that I assign a value to an array, and then i remove that value from the ArrayList. You can see I have put the System.out.println there, to see what it does, and this is the output:

34
33
32
31
30
29
28
27
26
16
15
14
13
12
11
10
9
8
7
6
5
4
3
2
1
0

And then the error obviously:

Exception in thread "main" java.lang.IllegalArgumentException: n must be positive
    at java.util.Random.nextInt(Random.java:300)
    at domein.Spelbord.wijsGebiedskaartenToe(Spelbord.java:123)
    at domein.Spelbord.<init>(Spelbord.java:19)
    at domein.Spel.setWereldkaart(Spel.java:18)
    at domein.DomeinController.maakWereldkaart(DomeinController.java:39)
    at ui.Uc1.maakWereldkaart(Uc1.java:42)
    at ui.ConsoleApplicatie.behandelUc1(ConsoleApplicatie.java:67)
    at ui.ConsoleApplicatie.startSpel(ConsoleApplicatie.java:29)
    at StartUp.main(StartUp.java:12)
Java Result: 1

I don't have a clue of whats going on, maybe you guys have?

donfuxx
  • 11,277
  • 6
  • 44
  • 76
stijn.aerts
  • 6,026
  • 5
  • 30
  • 46
  • 1
    Your loop seems to run only 9 (3 * 3) times, how can it print out so much? – skiwi Feb 28 '14 at 21:04
  • The inner part of your loop runs 9 times, which removes 9 elements (35 -> 26). So the rest of the output (from 16 on) is the result of some other code. For better help sooner, try to provide a [complete example](http://stackoverflow.com/help/mcve). – assylias Feb 28 '14 at 21:05
  • Your last printout of `gebiedskaarten.size()` is `0`, which, when passed to `r.nextInt()`, is going to be passing a non-positive number (`0`) to `Random#nextInt`, hence the error that occurs in `java.util.Random.nextInt` (look at your stack trace). – ajp15243 Feb 28 '14 at 21:06
  • 1
    This is still not compilable code, to really 100% assess your problem we need more code... (*but definately not all your code!*) However I also believe that with the current answers on `Random#nextInt` you have enough material to start to get it working. – skiwi Feb 28 '14 at 21:16
  • Problem was I forgot the break statements – stijn.aerts Feb 28 '14 at 21:23

3 Answers3

4

It is because you do:

 r.nextInt(gebiedskaarten.size());

when gebiedskaarten.size() == 0

==> you try get a random number from 0 to 0 ==> Exception

Update: it also looks like you forgot the break; statements in your switch-case! I guess you want to be your code logic to be like that:

 switch(sector.getCode()) {
 case 1: /*do something*/ break;
 case 2: /*do something else */ break;
 //...
 default: //do some default action
 }

see also related question: Why do we need break after case statements?

Community
  • 1
  • 1
donfuxx
  • 11,277
  • 6
  • 44
  • 76
2

You are calling Random.nextInt() with a value of 0, which is not supported.

As you see from the output of System.out, the array finally has a size of 0, and you are passing the array's size to nextInt().

See also http://docs.oracle.com/javase/7/docs/api/java/util/Random.html#nextInt(int)

Andreas Fester
  • 36,091
  • 7
  • 95
  • 123
  • Yes but it isnt supposed to be 0 – stijn.aerts Feb 28 '14 at 21:07
  • Thats a different issue then - I see that you updated your code, but I think that it now contains too much irrelevant code - can you shrink it down further so that it contains the minimum code required to reproduce the issue (see http://sscce.org for some good reading)? At the end, the problem is most certainly that you call `remove` too often, so that you end up with a size of 0 - note the remark from @donfuxx about the missing `break`s! – Andreas Fester Feb 28 '14 at 21:13
1

I do not know why your loop runs so many times, while only 9 (3 * 3) times is expected, but assuming that the code you posted starts when gebiedskaarten.size() == 9, it does the follow for 9 iterations:

  • System.out.println(gebiedskaarten.size())
    • Prints the current size.
  • Gebiedskaart gbk = gebiedskaarten.get(r.nextInt(gebiedskaarten.size()))
    • First executes r.nextInt(...) with gebiedskaarten.size(), obtaining a new random value.
    • Then calls gebiedskaarten.get(...) with that random number and assigns it to gbk.
  • vakken[i][j].setGbk(gbk);
    • Calls setGbk(gbk) on the i, j entry of vakken.
  • gebiedskaarten.remove(gbk);
    • Removes gbk from the gebiedskaarten collection, reducing the size by one.

Where it goes wrong, is that a certain point, gebiedskaarten.size() becomes 0, and you try to obtain a new random via r.nextInt(...) on 0, which is not possible, as it needs to be strictly positive.
Note however that calling gebiedskaarten.get(0) is valid, so you have an one-off error.

I also strongly suggest you to move from Dutch naming conventions to English naming conventions, as it will help you a lot in either your career future and in posting questions online.

Also, for completeness' sake, as @donfuxx has pointed out, your switch statements are not conform standards, which lead to errors in 99% of the time, it should be such:

switch (variable) {
    case 1:
        //do interesting things
        break;
    case 2:
        //do interesting things
        break;
    case 3:
        //do interesting things
        break;
    default:
        //either allowable base case, or may-not-happen error case
        break;

As you see I have added a break at the end of every case, this is neccessary to prevent it from falling through the statements. If there is no break at the end of case 1, then case 2 will also get executed.

skiwi
  • 66,971
  • 31
  • 131
  • 216