• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

code cleanup request - Bingo Cards

 
Troy Martin
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 50
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
 
Victor M. Pereira
Ranch Hand
Posts: 50
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
After you finish the java book read Clean Code of Robert C. Martin and Refactoring of Martin Fowler
 
Troy Martin
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 48381
56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 10087
55
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Troy Martin
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 48381
56
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 808
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 10087
55
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.

 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic