Get your CodeRanch badge!*
The moose likes Java in General and the fly likes If in a While not working Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Java in General
Bookmark "If in a While not working" Watch "If in a While not working" New topic
Author

If in a While not working

Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Hi, i've made this code to make a card, and i cant understand why the if within the while doesn't seem to be working.
I've even put in a system.print so that i'd know its entering it, but it misses it, if you could let me know where im going wrong it would be really helpful.

Basically this code is to make a set of 16 non duplicating numbers between 1 and 90, put them in array blocks of 10 which are sorted and make sure each array has atleast 1 number and no more than 3.

It could probably be done a lot better, but this is the only way i know how, if my logic is also wrong anywhere it'd be great if you could point it out and a fix.

Thanx in advanced.
Jon.



"It isn't until you've lost everything that your free to do anything" Tyler.
Ryan McGuire
Ranch Hand

Joined: Feb 18, 2005
Posts: 988
    
    1
What does it print? Anything?

Ryan
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Oh sorry, didn't think to show a print out of results.
It prints out the following.

12
47
64
54
25
59
29
34
16
32
53
3
54
26
34
19
72
47
9
[3, 9]
[12, 16, 19]
[25, 26, 29]
[32, 34]
[47]
[53, 54, 59]
[64]
[72]
[]
16
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24166
    
  30

There's too much code here to try to follow it all, but I'll point out one major problem, and let you see what happens if you fix it.

There are two different variables named currentRndNum. One is a member variable; the checkArrays() routine uses this one. But in the generateRndNum() routine, there's a local variable by the same name which shadows it. You assign a value to the local, which is never used for anything, and the member stays 0, forever. By the looks of the complex logic in checkArrays(), it's expecting currentRndNum to change, but it never will. Therefore I expect you're seeing this program go into an infinite loop on the first call to checkArrays() -- am I right?

Why use a member variable at all -- why not have the generateRndNum function simply return the random number? Then the caller can assign it to its own local variable. This is always preferred style compared to the "back door" approach of using a member as if it were a global variable.


[Jess in Action][AskingGoodQuestions]
Henry Wong
author
Sheriff

Joined: Sep 28, 2004
Posts: 18118
    
  39

I would put in a print statement, that prints out the counter, in the while loop. I am not convinced that it is being incremented properly.

Henry


Books: Java Threads, 3rd Edition, Jini in a Nutshell, and Java Gems (contributor)
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Argh! Just figured out where i've been getting totally confused, i copied my card3 and made card3testdrive to test it, and i've been pressing the run, but because i've been using eclipse it's been running card3 the old version of this card before i put in the check array's part.
How stupid of me.

You were right about the infinite loop, it wasn't before, but is now im running the right class.
I can see what you mean about the shadowing, but im not totally sure what the other things meant you said since im not very techincal on java yet.
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24166
    
  30

This is all I meant:



The "good style" version is easier to understand, easier to test, easier to get right the first time, less prone to bugs... There are any number of reasons why it's better.
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
AHHHH, my eyes!

Some things I see that look pretty bad.

you have:

that is only used like this:

It doesn't need to be there. Should be

It is generally a bad idea to ever use the = operator in an if condition.

There is a large amount of duplicated code. This program can be written with probably about 1/10th the amount of code you have used. As an added benefit to reducing the code it becomes much easier to find errors.

I understand you are fairly new to this, and this is not meant to be mean, just some constructive critisism.
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
Here is some code you maybe help you see the light. It isn't complete, and I haven't compiled or tested it, but it should help you get off to a better direction.

Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Well i've done some editing to my code and it now fully works the way i want it to, but i agree its not very manageable and there is a lot of duplicate and redundent code which needs to be cleaned up.

Thank you for all the help and tips, i'll try to use the example you gave and apply it to my work, and hopefully learn how to code a bit better.

Here is my current output

counter at 0
new 12
counter at 1
new 54
counter at 2
new 73
counter at 3
new 41
counter at 4
new 20
counter at 5
new 10
counter at 6
new 17
counter at 7
new 18
counter at 7
new 11
counter at 7
new 58
counter at 8
new 43
new 28
counter at 10
new 32
counter at 11
new 66
counter at 12
new 86
counter at 13
counter at 13
new 49
counter at 14
new 30
counter at 15
new 40
[10]
[12, 17, 20]
[28, 30]
[32, 40]
[41, 43, 49]
[54, 58]
[66]
[73]
[86]
16
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
I tried doing what you showed but i came across an error im not sure how to fix, i also modified the code you gave a little to give the results i think i need better.

Below is the output and the first part of my code currently, i can see it makes the index fine, but when it checks to see if that array has a size bigger than 2 i get a null pointer exception which i dont really know what that means.
The only thing i can guess at is that its because the array has not been initilised or something, but i thought everything which it set at the top automatically gets vaules so in this case it would be null.

Output:
counter at 0
new 81
index = 8
java.lang.NullPointerException
at Card3.setRandomNumberToArrayBlocks2(Card3.java:45)
at Card3.setRndNumbers(Card3.java:28)
at Card3.main(Card3.java:332)
Exception in thread "main"

Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Ok with some messing around i managed to fix the null pointer exception problem, and i've done a lot of editing and came up with some code which is much much better.

Only thing is now i want to remove a1To10, a11To20 etc...
But i've tried loads of different ways and this code only seems to run if it has the arrays inside the other arrays.
Your infinite wisdom is much needed as im too stupid to figure out how

Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
You do have to have code that puts the Sets into the array such as:


although this is how I would do it.



It's important that you understand everything that's going on so if you have any questions post them.
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
I dont really understand that
I know constructors run when you type new on something and they help initialise something before its made, but thats about all.

The main thing i dont understand here is you wrote arraySet, did you mean arrayColumn for my example? and wouldn't it mean all the a1To10 references would go in the constructor?
And do i remove all the local and instance references to the arrays with the constructor in place?

Jon.
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
Quick lesson on constructors.

All classes have at least one constructor. If you do not type one the java compiler will put one in there for you.

is the same as

When you call

Card3 myCard = new Card3();

you are saying call the constructor in the class Card3 and constructors always return an instance of its class.

In this example we need to have the TreeSet objects created and placed into an array when the object is created because the other methods assume this is done.

Where I wrote arraySet that would be the same as the arrayColumn variable you had, although the name doesn't really matter as long as you know what it does.

As for all the a1To10 references, if you look at the rest of your code they are never used. They don't have to be created at all. If you go back and look at my last post.


How's that?
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Ok i understand what you mean now

I only have 2 more things i need to do to my card3 class, i've tried to do 1 of them which is to make each array have 4 numbers, and the last numbers are meant to be blank, so i've selected the number 99 for it since it will stay at the end.

I've done this code but i cant understand why its not working.
It looks to me like it should work, and the output shows the size and size2 vales correctly but it is only adding 1 99 to each array.
It also finishes before all the arrays count is 36, but i cant work out why these 2 things are happening.



Output:
size = 1
size2 = 3
size = 2
size2 = 2
size = 2
size2 = 2
size = 1
size2 = 3
size = 2
size2 = 2
size = 2
size2 = 2
size = 3
size2 = 1
size = 1
size2 = 3
size = 2
size2 = 2
[5, 99]
[13, 16, 99]
[26, 29, 99]
[31, 99]
[41, 47, 99]
[56, 59, 99]
[64, 65, 68, 99]
[76, 99]
[83, 85, 99]
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
It's working right, but remember what you are using for storage. Take a look at TreeSet and especially the first line in the description of the Set interface.
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Oh yes, i've just thought what you mean without even looking, they cant contain duplicate values, not sure why that slipped my mind, lol.

Ok, now last step is to try and put all these into a 2 dimensional array of 4x9, not sure how well this will go...
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
I was hoping someone could help me, i've been trying to find a way to display my arrays in a grid, but found out that you cant get the index of a set so have got stuck on my last section of code.

Below is my current code with the displayCard() still being developed.
What i wanted is for it to loop through each array and then each index in that array so that each array is displayed in a column going down one after another with a grid around them.

The 2 solutions i could think of to my problem were to:
1. change the treeset to another type of array probably list type and put a sort in somewhere.
Or
2. before the displaycard method to put each array into another array that keeps index position, probably list and then use these arrays in the displaycard.

To me the 2nd seems better, but im not sure how to do it, i think i need to use the .toArray but im not sure how and im not sure what type of list is best.

If anyone could help me it would be really great

Thanx in advance,
Jon.


[ May 17, 2005: Message edited by: Jon Hughes ]
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
Could you post what you want the result to look like, just use the code tags and type it in.
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
Thats how i want it to finally look like, but where here the gaps are is where the numbers 97, 98 and 99 are in the arrays, i'll just put in a if those numbers insert space code.

Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
I would create a new Array:

Do a for loop to fill the array with the iterator of each set. Then you can loop over the iterators Array and call next() to get the next item. Each loop through the array will be a row.
Jon Hughes
Greenhorn

Joined: Mar 22, 2005
Posts: 14
I tried what you said but i just couldn't figure out how to do it so i tried searching around so im not constantly bugging you for simple answers.

I've almost completed this class, but i cant figure out how to unwrap the int values from the new array. I searched around and found some code thats meant to do it, but it goes through each array and gives my variable the last part of each array and i cant figure out how change it so it will give me the right value at the right time.

Below i've only posted the changes i've made since before, and a sample of some of the output.



Output:-
------------------------------------------------------
Current int: 1
Current int: 10
Current int: 98
Current int: 99
nRandomNumber = 99
1
| 99 |Current int: 1
Current int: 10
Current int: 98
Current int: 99
nRandomNumber = 99
10
| 99 |Current int: 1
Current int: 10
Current int: 98
Current int: 99
nRandomNumber = 99
98
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: If in a While not working
 
Similar Threads
Duplicate problem
Non-static method cannot be refrenced from a static context
Sorting vectors... a nightmare?
Output from a class shown in GUI?
Can I read from a Vector and writer into a File..