patternjavaMinor
Very Simple UNO Game
Viewed 0 times
gamesimpleunovery
Problem
I made a simple text-based UNO game in Java. It lacks some features of the original game, but it's good enough for now. I'm new to Java, so I would like to receive as much feedback and suggestions as I can on my code.
The Unocard class:
```
import java.util.Random;
public class Unocard
{
public String color;
public int value;
private Random rand;
private String face;
public Unocard(int v, String c)
{
value = v;
color = c;
}
// Creates a random card
public Unocard()
{
rand = new Random();
value = rand.nextInt(28); // 108 cards in a standard Uno deck. Can be reduced to 27 (disregarding colors)
// Assigning value
if (value >= 14) // Some cards are more common than others
value -= 14;
// Assigning color
rand = new Random();
switch(rand.nextInt(4) )
{
case 0: color = "Red";
break;
case 1: color = "Green";
break;
case 2: color = "Blue";
break;
case 3: color = "Yellow";
break;
}
// If the card is a wild card
if (value >= 13)
color = "none";
}
public String getFace()
{
/* Returns the face of the card (what the player sees)
* Ex. [Red 5]
*/
face = "[";
if (color != "none")
{
face += this.color + " ";
}
switch(this.value)
{
default: face += String.valueOf(this.value);
break;
case 10: face += "Skip";
break;
case 11: face += "Reverse";
break;
case 12: face += "Draw 2";
break;
case 13: face += "Wild";
break;
case 14: face += "Wild Draw 4";
break;
}
face += "]";
return face;
}
// Determines if
The Unocard class:
```
import java.util.Random;
public class Unocard
{
public String color;
public int value;
private Random rand;
private String face;
public Unocard(int v, String c)
{
value = v;
color = c;
}
// Creates a random card
public Unocard()
{
rand = new Random();
value = rand.nextInt(28); // 108 cards in a standard Uno deck. Can be reduced to 27 (disregarding colors)
// Assigning value
if (value >= 14) // Some cards are more common than others
value -= 14;
// Assigning color
rand = new Random();
switch(rand.nextInt(4) )
{
case 0: color = "Red";
break;
case 1: color = "Green";
break;
case 2: color = "Blue";
break;
case 3: color = "Yellow";
break;
}
// If the card is a wild card
if (value >= 13)
color = "none";
}
public String getFace()
{
/* Returns the face of the card (what the player sees)
* Ex. [Red 5]
*/
face = "[";
if (color != "none")
{
face += this.color + " ";
}
switch(this.value)
{
default: face += String.valueOf(this.value);
break;
case 10: face += "Skip";
break;
case 11: face += "Reverse";
break;
case 12: face += "Draw 2";
break;
case 13: face += "Wild";
break;
case 14: face += "Wild Draw 4";
break;
}
face += "]";
return face;
}
// Determines if
Solution
Think small, Think one
What I mean is that breaking up things into smaller chunks that all do one thing and do it well helps in so many ways. Now granted it can be taken too far, but for the most part if you break things down into something sort of vague and build from there. For instance if you think of uno as a few things such as "Game Rules", "Cards", "Players", "Deck" then you would at a bare minimum start with 4 classes. Those 4 things would do one thing and do it well. Like lets take Game Rules. You could have started with it calling each player in order and asking them to take their turn. It could have kept track of the deck and gave a player a card when it was requested. It could have also shuffled the deck of predefined cards (like it sorta would have been if you buy the game). After each player it could have checked if said player won or not. This would have made the class kind of big, but now maybe you could have seen that the user input is what is bloating the class. So you could have pulled it out in a user input class of sorts.. so on and so forth. The point is that if you think of the individual moving parts of a system and start there and work your way down while keeping in the back of your mind that you want things to stay small, and they should be responsible for one thing and one thing only then your code would look very different than it does now and potentially easier to understand at first glance
Magic Numbers
Magic numbers are numbers or string literals (for example:
It's not much, but I think the concept of what i'm saying will make a big difference. Good luck.
What I mean is that breaking up things into smaller chunks that all do one thing and do it well helps in so many ways. Now granted it can be taken too far, but for the most part if you break things down into something sort of vague and build from there. For instance if you think of uno as a few things such as "Game Rules", "Cards", "Players", "Deck" then you would at a bare minimum start with 4 classes. Those 4 things would do one thing and do it well. Like lets take Game Rules. You could have started with it calling each player in order and asking them to take their turn. It could have kept track of the deck and gave a player a card when it was requested. It could have also shuffled the deck of predefined cards (like it sorta would have been if you buy the game). After each player it could have checked if said player won or not. This would have made the class kind of big, but now maybe you could have seen that the user input is what is bloating the class. So you could have pulled it out in a user input class of sorts.. so on and so forth. The point is that if you think of the individual moving parts of a system and start there and work your way down while keeping in the back of your mind that you want things to stay small, and they should be responsible for one thing and one thing only then your code would look very different than it does now and potentially easier to understand at first glance
Magic Numbers
Magic numbers are numbers or string literals (for example:
"none") make a person wonder. "What does 12 mean?" or "What does 13/14 do?". You asked that question to your self at one time because you put a comment in to tell yourself that 12 means that the card is a Draw 2. So instead of putting 12 why not make a class (abstract class if you want, or maybe an enum) that contains some these magic numbers it takes the guess work and the comments out of your code. I imagine it looking somethign like this in one caseswitch (topCard.value)
{
case CardValues.DrawTwo
System.out.println("Drawing 2 cards...");
draw(2,compdeck);
break;
case CardValues.Wild:
case CardValues.WildDrawFour:
//...code revmoed for clarity
System.out.println("You chose " + currentColor);
if (topCard.value == CardValues.WildDrawFour)
{
System.out.println("Drawing 4 cards...");
draw(4,compdeck);
}
break;
}It's not much, but I think the concept of what i'm saying will make a big difference. Good luck.
Code Snippets
switch (topCard.value)
{
case CardValues.DrawTwo
System.out.println("Drawing 2 cards...");
draw(2,compdeck);
break;
case CardValues.Wild:
case CardValues.WildDrawFour:
//...code revmoed for clarity
System.out.println("You chose " + currentColor);
if (topCard.value == CardValues.WildDrawFour)
{
System.out.println("Drawing 4 cards...");
draw(4,compdeck);
}
break;
}Context
StackExchange Code Review Q#124722, answer score: 5
Revisions (0)
No revisions yet.