[OCP 17 book] | [OCP 11 book] | [OCA 8 book] [OCP 8 book] [Practice tests book] [Blog] [JavaRanch FAQ] [How To Ask Questions] [Book Promos]
Other Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, TOGAF part 1 and part 2
That isn't long code; it is quite a small class. Asking for feedback means that is what you get, and it might not be nice to read. Read on at your peril!
Line 1: Only use that package name if you own that website. Have a look in the Java™ Tutorials.
Lines 3‑5: The indentation in that comment is inconsistent. It should look like this:-
The line‑end comments in the class body are inappropriate; some of them only tell you what you know already. Those in line 15 and 23 are incorrect because you don't have constructors there. I think you may do well to revise what a constructor is. The comments on public methods should be converted to documentation comments.
Carey Brown wrote:You have way too much redundancy in all your print blocks. Make a single print method, passing in enough parameters to effect the visual output (e.g. who wins) as necessary.
Not fond of 'p' vs 'p1'. Would have preferred 'p1' and 'p2'.
So why are you only returning name from toString()? Shouldn't you include the fingers and the guess?Ansamana Sankarray wrote:. . . I remade the code from zero, including . . . the toString() method . . .
Kudos for being prepared to take criticism and make changesAnsamana Sankarray wrote:. . . I have remade that class from scratch too. . . . .
That's a pleasure.Thanks
You don't need to search; we can tell you whether you need to close a Scanner:-Ansamana Sankarray wrote:. . . the scanner was not closed, I've searched about it but nothing clear . . .
Paul Folder wrote:Hello,
Some observations in addition to what others have posted:
-Reading the description of GuessAndWin confused me. I thought guessing a number and guessing a finger is two completely separate things and didn't understand the connection and why both are needed. Isn't each player just guessing 2 numbers; an own and a sum?
-All of the methods in GuessAndWin are static at the moment.
-In main(), lines 139-140, two new Player objects are instantiated. Their names are then handed over to playGame(), but there, on lines 33-34 two new Player objects are instantiated again. Consider either handing over the Player objects themselves to playGame(), or instantiating them only in playGame(). Perhaps there will be a use for setters in Player again.
-Some of the printed text could be stored in constants. I see lots of "line breaks" ("-----------------------------------------").
Keep going!
Did you see how Paul cut 87% off of his electric heat bill with 82 watts of micro heaters? |