Bear Bibeault wrote:Surely you can provide more detail than "my code crashes".
Jon
Stephan van Hulst wrote:Hi Jon. I have several comments about your code. These are mostly personal preferences, but I strongly urge you to consider them:
- Put fields at the top of your code, and methods below them. It's hard to find out what your code is referring to when you have to scroll down to find the fields.
- Don't make one line methods like getSomething() {return something}. It's hard to distinguish them from fields (especially when generic types are involved).
- Place opening braces either on the same line as the method signature, or on a new line below it. Match the closing brace with the column of either the opening brace or the method signature. Choose one, and stick with it.
- Make all your fields private. Always. There is rarely a good reason to make them more visible than that.
- Don't use protected. There is rarely a good reason to make members protected, and only after long and hard consideration. Always prefer private and default access.
- Make internal classes static. This includes enums.
- Don't overuse inheritance. It's a powerful mechanism, but it violates encapsulation, a far more important concept. Conceptually a deck of cards is not a card, so don't make Deck extend Card. Conceptually, a dealer isn't a player, so don't make Dealer extend Player. As a matter of fact, make all your classes final unless you have a good reason not to.
- Try not to use identifiers already used by the standard API. Especially not well known class names like Number. This will confuse anyone who reads your code. Number is the supertype of all numeric classes, like Integer, Float, etc. Use a name like Rank instead.
- Limit the use of byte, short and long if you can. This will avoid the need to cast ints, which is the default integer type.
- Card is a prime example of a class you can make immutable. Right now there is a probable bug in it; namely a public constructor without parameters that does nothing. Calling this constructor will return a card that is in an inconsistent state.
- Consider overriding equals() and hashCode(). This will make your Card class a lot more easy to use.
Jon
Stephan van Hulst wrote:
- Consider overriding equals() and hashCode(). This will make your Card class a lot more easy to use.
Jon
Jon Camilleri wrote:With regards the design of Deck and Cards, I thought I would extend Deck as a set of Cards, would you suggest other data structures to implement a set of Cards, perhaps an array of Cards[], rather than a parent? However taking the array[] approach, which is quite a problematic data structure from my experience in Java, would go against the notion of designing in terms of Objects (OOP), rather than rigging functional programming within OOP design. What is your advice?
Actually I dropped in the blank (inconsistent) constructor for Card simply to work around the need for Deck to have a constructor for Card without having to pass the Suit and the Rank.
My application does not really require a hashCode at this stage, however I would like to know whether my classes make use of hash codes (so far I haven't made any use as far as I know), thus implementing an efficient hashCode would make my application more efficient?
On the other hand, "this method must be overridden in every class that overrides the equals method."
Source from http://www.technofundo.com/tech/java/equalhash.html.
Sun have already encoded a hashCode, hence, I could theoretically speaking get away with it, right? Why would I override functionality written by Sun Microsystems software engineers, and, re-invent the wheel?
Having had second thoughts, I have updated a hashCode() that generates a random number, however, reliability of the uniqueness of the hashCode() is dependent on the probability of the Random class generating the same number. The hashCode that was suggested was based on hashCode() that should be generated from enums Suit and Rank, within the Card class, and, it seemed too intriguing to follow that error-prone logic, however, I am open to suggestions in this regard.
Having had second thoughts, I have updated a hashCode() that generates a random number, however, reliability of the uniqueness of the hashCode() is dependent on the probability of the Random class generating the same number. The hashCode that was suggested was based on hashCode() that should be generated from enums Suit and Rank, within the Card class, and, it seemed too intriguing to follow that error-prone logic, however, I am open to suggestions in this regard.
Jon
Stephan van Hulst wrote:Again, look at the example I provided. It does exactly this. Read it, test it, and tell us if there's something you don't understand.
Jon
Jon Camilleri wrote:
Stephan van Hulst wrote:Again, look at the example I provided. It does exactly this. Read it, test it, and tell us if there's something you don't understand.
Yes indeed, it's correct, however, somehow I thought it was incorrect, because either Eclipse going out of sync or some copy and paste error (my bad), was loading a bunch of errors and the hashCode() was not being returned for the Card class.
Jon
Something must be done about this. Let's start by reading this tiny ad:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
|