Win a copy of Re-engineering Legacy Software this week in the Refactoring forum
or Docker in Action in the Cloud/Virtualization forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Random Card Problem Check

 
Hai-Tan Le
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hey all, I have a program that repeatedly flips a coin (Heads or Tails) until 3 consecutive Heads occur, at which point the program will display how many flips it took. I am new to Java programming and was hoping if someone could look over my program. To me, the program looks a bit messy, so if there is a different way that you would have written it more cleanly, please let me know. Thanks

 
Henry Wong
author
Marshal
Pie
Posts: 20902
76
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

1. I think it is a good idea to always use equals() to compare strings. In this case, it works because you are only using string literals -- and the string pool is helping, but this will quickly break if you do any string operation.

2. Instead of having three embedded conditionals, can you get it to work with another counter? Heads would increment it, and tails will set it back to zero. And of course, you would need a check to exit the loop when the count goes to three.

 
Stephan van Hulst
Bartender
Pie
Posts: 5415
52
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You shouldn't write for loops like that. To most programmers it looks like an infinite loop, because i is almost always smaller than i +1.

Make a more descriptive sounding while loop, like: while (streak < 3).

Your trueFalse() method is named badly. You would expect it to return a String that reads "true" or "false".

You should also probably put all your fields at the top of your class. I had to look around for a bit to find rgen.
 
Matthew Brown
Bartender
Posts: 4565
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't like the idea of using Strings to represent "Heads" and "Tails". I'd do one of two things:

- Just use a boolean - e.g. true for heads, false for tails

- If you want it to be more readable (so you don't need to remember which is which), define an enum with HEADS and TAILS as the two values

If you were going to continue using Strings, at least define a couple of constants for them. As things are you're risking mispelling it somewhere and creating a very hard to find error.
 
Hai-Tan Le
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks everyone, I really appreciate it.
 
Suletea Dorin
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


I didn't test or compile this but it should work (and it's not the best implementation)

1) Try using enumerations instead of String or Integers or whatever when you have a limited number of possibilities
2) Try avoiding infinite loops , even if you use a break statement in there (good habit)
3) If you must use an infinite loop (who knows) go with while(true) if you don't have statements after the loop or you will get an "unreachable code compile error".
4) Be afraid of nested "if's" they are evil. In big applications it can be a pain in the you know where.
5) Don't use the run() method if you are not working with threads it's confusing (good practice again) does program class extend thread?


I hope that helps.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic