• Post Reply Bookmark Topic Watch Topic
  • New Topic

My sad little program can't seem to count cards right  RSS feed

 
Jeremy Graham
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's my Card object:



Here's my Deck object:


And here's the main method:


I'm consistently getting wrong counts. For example, I get
6 of spades
7 of hearts
8 of hearts
7 of diamonds
8 of diamonds

but it counts 2 cards as non Spades (as oppoesed to 4).

Any ideas?
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Let's say you have 5 cards in your hand, call them A, B, C, D, E, and you remove the card at index 2.

What does your hand look like now?

When you go around for the next pass at your loop, which card will you be looking at?

Aside from that, there are a number of problems in your code.

1. you should be using enums, not ints for rank and suit.

2. Even if you're not allowed to use enums, you shouldn't be constructing each card explicitly. Use loops.

3. Use java.util.Random.nextInt(), not Math.random().

4. Never use get(i) to iterate over a Collection. Use a foreach loop, or, if you want to modify the Collection during the iteration, use an Iterator. (That would fix the non-spade-count problem, if I read that bit of your code correctly.)
 
Jeremy Graham
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I can't seem to figure out how to make unique objects for an arraylist using a for loop... Any ideas how?

Thanks
 
dennis deems
Ranch Hand
Posts: 808
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Graham wrote:I can't seem to figure out how to make unique objects for an arraylist using a for loop... Any ideas how?

Thanks

If the instantiations occur inside a loop, they will be unique. No additional effort is required from you to make it so. Have you learned about scope yet? Every set of {} braces defines a scope, and anything local to that scope is unique. If I write and copy/paste that block ten times inside a method, then I have instantiated ten unique String objects. Each of those objects is only visible inside its scope, which is why I don't get an error when I reuse the variable name unique. We can arbitrarily define blocks inside a method, though it's usually not a good idea. However, structures like loops and if/else alternations define their own scope blocks. If I take my String instantiation and put it inside a loop that iterates ten times, then it is just as if I had copied and pasted the block ten times: ten unique Strings are created.

So: you already know how to instantiate an object in a method and add it to a list. Doing it inside a loop isn't any different. Make a for loop that iterates over the ranks, and inside that loop make another, nested loop, that iterates over the suits. Instantiate a card of that rank and suit, then add it to the deck -- much like you already do. The only real difference is that instead of 52 instantiations you only have to write one.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dennis Deems wrote:
So: you already know how to instantiate an object in a method and add it to a list. Doing it inside a loop isn't any different.


Well, there will need to be special handling for J, Q, K, A, but there are various ways to deal with that.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeff Verdegan wrote: . . . special handling for J, Q, K, A, but there are various ways to deal with that.
And making Suit and Rank into enums makes it much easier to find one of those “various ways”.
 
Alan Smith
Ranch Hand
Posts: 185
Firefox Browser Linux Netbeans IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Give your card class a list for ranks and a list for suits ie:


then use nested for loops to populate your deck with cards based on the rank and suit arrays. Make sure the card class has rank and suit attributes and be able to set them using the constructer ie:


that should get you started.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I still think the earlier suggestion about enums was good. If you are up to reading the Java Language Specification and about the Enum class, that will provide more useful information.
 
dennis deems
Ranch Hand
Posts: 808
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:I still think the earlier suggestion about enums was good. If you are up to reading the Java Language Specification and about the Enum class, that will provide more useful information.

Enums would definitely simplify this problem. But I don't think the JLS makes beginner-friendly reading. The Java Tutorial you linked to has everything the beginner needs, and the exercise which follows the Enum chapter is actually this very problem: to represent a deck of cards with enums.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think it makes sense to do it without enums first, to learn how it can be done with looping and arrays, and then to do it again with enums, as a compare-and-contrast exercise.
 
dennis deems
Ranch Hand
Posts: 808
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeff Verdegan wrote:I think it makes sense to do it without enums first, to learn how it can be done with looping and arrays, and then to do it again with enums, as a compare-and-contrast exercise.
This is a great idea.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Graham wrote:but it counts 2 cards as non Spades (as oppoesed to 4).

So, you have an array of 4 things (Suits). What are their indexes?

Winston
 
Jeremy Graham
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you Dennis for your reply. I know I've tried doing that sort of thing in the past, but I apparently was always doing something wrong.

I'm still in the process of figuring out what I want to do with enums. I've tabled it for now, but I'll get back to it.

What is stumping me right now is Jeff's suggestion that I use java.util.Random.nextInt() instead of Math.random(). In trying to use it I'm getting the error: "non-static method nextInt(int) cannot be referenced from a static context". Here's all my code again.

My Card object:
public class Card {


My Deck2 object (keeping the original Deck object for now):


And the main method:


The only static method I see is in that main method.

Help?
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dennis Deems wrote:. . . But I don't think the JLS makes beginner-friendly reading. . . .
No, it doesn’t. That’s why I said, “If you are up to reading . . .”
 
Nigel Browne
Ranch Hand
Posts: 704
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Jerremy,

Your Card class doesn't have a constructor. I would create a card with either suit and rank, or name as in the case of a joker.
I would get rid of your set methods as once a card is created it doesn't change its' value or suit.
I would also change your getName method into a toString() method.

I highly recommend that you read the java tutorial regarding enums as your Deck class could be reduced down to around 20 lines of code. Also do the deal and showHand methods really belong in the Deck class?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Graham wrote:
What is stumping me right now is Jeff's suggestion that I use java.util.Random.nextInt() instead of Math.random(). In trying to use it I'm getting the error: "non-static method nextInt(int) cannot be referenced from a static context".

You wrote:

Line 24 is the problem. "non-static method nextInt(int)" means that the nextInt method is an instance method. That means you can only call it on an instance of the Random class. How do you get an instance? You use the new keyword, of course.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Graham wrote:Here's my Card object:
...

One of the complaints about Java which other JVM languages like Scala and Groovy try to avoid is its verbosity in declaring even the simplest of classes. You might want to consider this: Once created, does a Card object ever change its suit or rank? IMO, it doesn't. These are all intrinsic, immutable properties of a Card once it has been instantiated, unless you are simulating a magic card trick where the Card will transform from the Queen of Hearts to the King of Spades right before your eyes

If you agree that a Card should not be able to change its suit or rank once it is instantiated, then you should use a constructor that takes those values and assigns them to final fields. You can then choose to just make the final fields public or provide only getters for those fields, no setters because those don't make sense.

EDIT: Now I feel a little icky... seems I just kicked a Zombie, too
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:EDIT: Now I feel a little icky... seems I just kicked a Zombie, too

We've all done it.

My only contribution is that I've been playing bridge at club level for about 30 years, and I still can't count cards right.

Winston
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!