File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Beginning Java and the fly likes code cleanup request - Bingo Cards Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "code cleanup request - Bingo Cards" Watch "code cleanup request - Bingo Cards" New topic
Author

code cleanup request - Bingo Cards

Troy Martin
Greenhorn

Joined: Mar 03, 2012
Posts: 4
Hello there

I'm pretty new to java and have been going through a book attempting to learn the language. I took one of the examples from the book I'm using (they were teaching how to do a tic-tac-toe board) and expanded upon it a bit to make a bingo card generator. It all seems to be working just fine, I was just wanting to see if there is a cleaner way I could write all this (especially the last part where I enter the values for the array... I'm pretty sure I could do another 'for' loop for the second array but wasn't entirely sure how to go about that considering the numbers change).

Thanks in advance!!

Victor M. Pereira
Ranch Hand

Joined: Mar 02, 2012
Posts: 50
I'm not really familiar with bingo so I don't know if your class is correct. What I can help you with is code smells.

Watch your main and you'll see the recurrent use of the for, while, generation and some number.



And easy way to split method is describing what it does with TO.

To doSomething I need to updateSomething then, calculateSomthing and finally printSomething

doSomething = method name
to then finally = private methods


regards,
Victor M. Pereira
Victor M. Pereira
Ranch Hand

Joined: Mar 02, 2012
Posts: 50
After you finish the java book read Clean Code of Robert C. Martin and Refactoring of Martin Fowler
Troy Martin
Greenhorn

Joined: Mar 03, 2012
Posts: 4
Thank you very much. I'm very new to OOP programming and quite a bit different than what I'm used to; I greatly appreciate the assistance!

I'll look over this in greater detail later and ask any further questions I may have.

Thanks also for the book suggestion, I'll be sure to check that out once I finish my current read.

Appreciated bud!
Troy Martin
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39393
    
  28
Welcome to the Ranch

I do not think you have got any object orientation at all, I am afraid. Nowhere do you have a card object or similar. You can tell that by the liberal use of the keyword static.
The main method is much much too long. The ideal length of a main method is one statement. I don’t like break; but others will disagree with me.
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 8008
    
  22

Troy Martin wrote:Thank you very much. I'm very new to OOP programming and quite a bit different than what I'm used to; I greatly appreciate the assistance!

No probs. We were all new once, but I definitely concur with Campbell.

Think about putting your bingo card logic in a separate class (and whatever you do, DON'T mix the display logic with the 'working' code).

Another suggestion: Your card seems to include dimension logic in it, which I suspect is a bit of a red herring (again, dimensions have to do with display).
I think, if it was me, I'd just look at each card as a set of numbers from (say) 1-50, with random ones blanked out. If you turn that around, you can also look at it as a predetermined number (say 25) of random distinct numbers between 1 and 50, which will make your List very easy to search (particularly if it's sorted). How you display those numbers is an entirely different issue.

I would also suggest looking at the Collections.shuffle() method. I think you'll find it very useful for this program.

Winston

PS: What I'm really interested in is your "bingo calling" database. You know...clickety-click, two fat ladies, etc.


Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
Troy Martin
Greenhorn

Joined: Mar 03, 2012
Posts: 4
Thanks again for the input! I actually ended up getting a different book which explains things FAR better (especially objects / classes) so I have a much greater understanding of the whole process. I'll be re-writing this in the near future after getting a bit more practice with objects. Just from what I learned over the course of a few hours, I can tell my above code is horrendous (I understand why you must all have cringed at my code... DEFINITELY not OO at all.)

At any rate, really appreciate all the suggestions and I'll check back here when I get around to re-writing the code.

@Winston: I'll be sure to mention the ladies when I get around to writing the 'calling' ;)

Best Regards,
Troy
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39393
    
  28
Troy Martin wrote: . . . @Winston: I'll be sure to mention the ladies when I get around to writing the 'calling' ;) . . .
I can remember the days when saying, “ladies,” was sufficient to get a reply of, “Male chauvinist pig!” So I shan’t be prissily PC about it
dennis deems
Ranch Hand

Joined: Mar 12, 2011
Posts: 808
Winston Gutkowski wrote:
Another suggestion: Your card seems to include dimension logic in it, which I suspect is a bit of a red herring (again, dimensions have to do with display).
I think, if it was me, I'd just look at each card as a set of numbers from (say) 1-50, with random ones blanked out.


Not sure I agree a hundred percent with this. A Bingo card isn't merely a set of numbers. It is five sets of numbers.
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 8008
    
  22

Dennis Deems wrote:Not sure I agree a hundred percent with this. A Bingo card isn't merely a set of numbers. It is five sets of numbers.

Actually, I took a look at the Wiki pages on Bingo after this thread, and it depends whether you're talking about American or Commonwealth Bingo. In the latter, the "card" is just a set of 9 columns and 3 lines that get filled as numbers get called, so it's really quite different.

However, I take your point; I was just trying to make sure that Troy didn't get the display logic confused with game logic.

Winston
Troy Martin
Greenhorn

Joined: Mar 03, 2012
Posts: 4
Just for the record, (I never about Commonwealth Bingo *learns something*) my code referred to the American version of Bingo. Still working through this book but will be updating my horrendous example above shortly.
Thanks!
Troy
John Webb
Greenhorn

Joined: Jun 05, 2012
Posts: 2
Hey Troy i would advice also to change the number 15 to be a final, say "MAX_RANGE=15" and maybe after you finished the book
(i think i know which book it is although tic-tac-toe is a recurring example in all coding books) try to extend the bingo generator (use inheritance and generics for practice)
to something sites like bingocardgenerator do which is a bingo generator with terms or pictures instead of numbers that go to 75. I think it would
be a really good exercise for you.

 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: code cleanup request - Bingo Cards