Thread: My code is not working

Hi
I was wondering if someone can help me out with a bit of code.
I cant get my loop to work properly, in particular the incrementing of vehicle [0]
Everytime I run the loop it skips vehicle[0] but after i punch in 1 for a second time it starts to increment. the other counters are working fine except for this [0]
Can anyone tell me how to fix this problem?

Thanks

  int usernumber;
		int vehicle[]={1,2,3,4,5,6,7}; 
		int counter[]={0,0,0,0,0,0,0};
 
		Scanner scanner = new Scanner(System.in);
		usernumber = scanner.nextInt();
 
 
		while (usernumber <= 7 && usernumber >= 1)
		{
			usernumber = scanner.nextInt();
 
 
		   if (usernumber == vehicle[0])
		   {
		    counter[0] = counter[0] + 1;
	        }
	       if (usernumber == vehicle[1])   
	       {
	        counter[1] = counter[1] + 1;
	        }
	       if (usernumber == vehicle[2])   
	       {
	        counter[2] = counter[2] + 1;
	        }
	       if (usernumber == vehicle[3]) 
	       {
	        counter[3] = counter[3] + 1;
	        }
	       if (usernumber == vehicle[4])
	       {
	        counter[4] = counter[4] + 1;
	        }
	       if (usernumber == vehicle[5])
	       {
	        counter[5] = counter[5] + 1;
	        }
	       if (usernumber == vehicle[6])
	       {
	        counter[6] = counter[6] + 1;
	        }
		}

In my first 'proper' programming job, we had to 'check out' source to edit by going into the library, finding the folder on the hanging racks that had the continuous-sheet print out of the last change, physically taking it back to our desks and then debugging the problem section by hand. To do that, there was a template (a plastic sheet with holes in) that you put over the paper, drew a few lines to make a simple table and wrote some standard headings on it. Then you'd have to write the current line number in the left column and write the values of your variable-of-interest in the rest of the row and at the end of each row a note of what changed on that line. You'd repeat that until you could demonstrate the bug in pencil marks. Only then could you edit the code and pass the lot onto the testers.

What we wouldn't have given for a web forum! I was looking at your code a day or two ago - you were asking for a 'short version', right? There are some obvious ways of making this code smaller. Your logic is in an awkward order. After you instantiate the Scanner, you read a number. The next line (the while) you validate that number, and then you read a number which you don't validate. And then you use that number to increment your counts. I'll stop there, I guess you can see the problem now!

There are some obvious ways of making this code smaller.

Namely a loop.

Namely a loop.

I was thinking along the lines of 'looking up' the vehicle references rather than hard-coding them. The code:

if (usernumber == vehicle[0])
{
counter[0] = counter[0] + 1;
}
if (usernumber == vehicle[1])
{
counter[1] = counter[1] + 1;
}
if (usernumber == vehicle[2])
{
counter[2] = counter[2] + 1;
}

uses the contents of the vehicle array which appear to me to be static (the vehicle ids don't change and there are no gaps - which sounds plausible, if not very flexible). The vehicle array is basically an index map from vehicle numbers to counter index. So the code is like this:

if (usernumber == 1)
  counter[0 /* usernumber - 1 */] = counter[0 /* usernumber - 1 */] + 1;
else if (usernumber == 2) // else because only one expression at a time can be true
  counter[1 /* usernumber - 1 */] = counter[1 /* usernumber - 1 */] + 1;
else if (usernumber == 3) // no brackets needed for only one statement
  counter[2 /* usernumber - 1 */] = counter[2 /* usernumber - 1 */] + 1;

... since the counter index is always 1 less than the entered usernumber, the cascade of if statements can be reduced to this:

if (usernumber > 0 && usernumber <= counter.length)
  counter[usernumber - 1]++; // myInt++ is the same as myInt = myInt + 1

OP can even put an else clause on that input-validating-if to break out of the loop (there isn't always a nice concise loop-terminating condition for user-input loops, so I'd use a 'while (true)' here) if the input is not valid. Am I spoonfeeding yet?

Hey Sean
Is there a way of incrementing that first initial input without re doing the whole thing. I have tried everything I could but still no worky

Thanks so much for your help

Your first getInt from Scanner is never used - the value of usernumber is immediately re-assigned on entry into the loop. You could just delete the Scanner.getInt line before the loop. If you're worried about deleting code, just 'comment it out' - put a // at the start of the line or wrap the whole line in /* and */

When you do that, your while loop won't run because Java initialises int data members to zero if you don't initialise them to something else. You could kludge your code by initialising usernumber to 1-to-7, but that would be nasty. What your loop should really look like is (pseudocode)

loop
    get input
    if input is not valid
        exit the loop
    do something with the input
end loop

because validation has to follow input, it's hard to put the validation tests in the condition for the while loop. There is a way of doing it that I sometimes use and I suspect a lot of Java coders use (put the input assignment inside the condition), but it's ugly. My preference for cases like this is to 'loop forever' - use 'while(true)' - get the input and break on failed validation.