Choose random card from a deck

Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
1
down vote
favorite
still new to java and i am trying to improve my understanding and use of OOP concepts.here i am supposed to print the name of a randomly selected card from a deck of 52 cards.
Program:
/*
* RandomCard.java
* -------------------
* Displays the name of a card randomly chosen from a complete deck of 52
* playing cards.
*/
import java.util.Random;
public class RandomCard
/* Instance Variables */
private Random rgen = new Random(); /* random generator */
/**
* Display the name of a randomly selected card from a deck of 52 cards
*/
public static void main(String args)
System.out.println("This program displays a randomly chosen card");
Card randomCard = selectCardFromDeckRandomly();
System.out.println(randomCard);
/*
* Randomly select a card from a card deck
* @ return The selected card
*/
private Card selectCardFromDeckRandomly()
String rank = selectRanomCardRank();
String suit = selectRandomCardSuit();
return (new Card(rank, suit));
/*
* Randomly select a card rank
* @ return the selected card rank
*/
private String selectRanomCardRank()
String cardRank;
int r = 1 + rgen.nextInt(13);
switch (r)
case 1:
cardRank = "Ace";
break;
case 2:
cardRank = "2";
break;
case 3:
cardRank = "3";
break;
case 4:
cardRank = "4";
break;
case 5:
cardRank = "5";
break;
case 6:
cardRank = "6";
break;
case 7:
cardRank = "7";
break;
case 8:
cardRank = "8";
break;
case 9:
cardRank = "9";
break;
case 10:
cardRank = "10";
break;
case 11:
cardRank = "Jack";
break;
case 12:
cardRank = "Queen";
break;
default:
cardRank = "King";
break;
return cardRank;
/*
* Randomly select a card suit
* @ return The selected card suit
*/
private String selectRandomCardSuit()
String cardSuit;
int s = 1 + rgen.nextInt(4);
switch (s)
case 1:
cardSuit = "Clubs";
break;
case 2:
cardSuit = "Diamonds";
break;
case 3:
cardSuit = "Hearts";
break;
default:
cardSuit = "Spades";
break;
return cardSuit;
class Card:
/*
* Card.java
* -------------
* Creates a playing card of certain Rank and Suit.
* This class is part of exercises 1 solution.
* This program is intended as a practice on programming methodologies.
*/
public class Card
/* Instance variables */
private String rank; /* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
private String suit; /* The card suit : Clubs, Diamonds, Hearts, Spades */
/* Constructor */
/**
* Creates an instance object of class Card with the specified rank and suit
* @param rnk The rank of the card
* @param sut The suit of the card
*/
public Card(String rnk, String sut)
rank = rnk;
suit = sut;
public String toString()
return (rank + " of " + suit);
Note: still not in a position to use arrays.
Evaluation points:
- Encapsulation
- Top-Down design (refactoring)
- Readability and complexity reduction
- Object oriented
- Documentation
java beginner random playing-cards
add a comment |Â
up vote
1
down vote
favorite
still new to java and i am trying to improve my understanding and use of OOP concepts.here i am supposed to print the name of a randomly selected card from a deck of 52 cards.
Program:
/*
* RandomCard.java
* -------------------
* Displays the name of a card randomly chosen from a complete deck of 52
* playing cards.
*/
import java.util.Random;
public class RandomCard
/* Instance Variables */
private Random rgen = new Random(); /* random generator */
/**
* Display the name of a randomly selected card from a deck of 52 cards
*/
public static void main(String args)
System.out.println("This program displays a randomly chosen card");
Card randomCard = selectCardFromDeckRandomly();
System.out.println(randomCard);
/*
* Randomly select a card from a card deck
* @ return The selected card
*/
private Card selectCardFromDeckRandomly()
String rank = selectRanomCardRank();
String suit = selectRandomCardSuit();
return (new Card(rank, suit));
/*
* Randomly select a card rank
* @ return the selected card rank
*/
private String selectRanomCardRank()
String cardRank;
int r = 1 + rgen.nextInt(13);
switch (r)
case 1:
cardRank = "Ace";
break;
case 2:
cardRank = "2";
break;
case 3:
cardRank = "3";
break;
case 4:
cardRank = "4";
break;
case 5:
cardRank = "5";
break;
case 6:
cardRank = "6";
break;
case 7:
cardRank = "7";
break;
case 8:
cardRank = "8";
break;
case 9:
cardRank = "9";
break;
case 10:
cardRank = "10";
break;
case 11:
cardRank = "Jack";
break;
case 12:
cardRank = "Queen";
break;
default:
cardRank = "King";
break;
return cardRank;
/*
* Randomly select a card suit
* @ return The selected card suit
*/
private String selectRandomCardSuit()
String cardSuit;
int s = 1 + rgen.nextInt(4);
switch (s)
case 1:
cardSuit = "Clubs";
break;
case 2:
cardSuit = "Diamonds";
break;
case 3:
cardSuit = "Hearts";
break;
default:
cardSuit = "Spades";
break;
return cardSuit;
class Card:
/*
* Card.java
* -------------
* Creates a playing card of certain Rank and Suit.
* This class is part of exercises 1 solution.
* This program is intended as a practice on programming methodologies.
*/
public class Card
/* Instance variables */
private String rank; /* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
private String suit; /* The card suit : Clubs, Diamonds, Hearts, Spades */
/* Constructor */
/**
* Creates an instance object of class Card with the specified rank and suit
* @param rnk The rank of the card
* @param sut The suit of the card
*/
public Card(String rnk, String sut)
rank = rnk;
suit = sut;
public String toString()
return (rank + " of " + suit);
Note: still not in a position to use arrays.
Evaluation points:
- Encapsulation
- Top-Down design (refactoring)
- Readability and complexity reduction
- Object oriented
- Documentation
java beginner random playing-cards
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21
add a comment |Â
up vote
1
down vote
favorite
up vote
1
down vote
favorite
still new to java and i am trying to improve my understanding and use of OOP concepts.here i am supposed to print the name of a randomly selected card from a deck of 52 cards.
Program:
/*
* RandomCard.java
* -------------------
* Displays the name of a card randomly chosen from a complete deck of 52
* playing cards.
*/
import java.util.Random;
public class RandomCard
/* Instance Variables */
private Random rgen = new Random(); /* random generator */
/**
* Display the name of a randomly selected card from a deck of 52 cards
*/
public static void main(String args)
System.out.println("This program displays a randomly chosen card");
Card randomCard = selectCardFromDeckRandomly();
System.out.println(randomCard);
/*
* Randomly select a card from a card deck
* @ return The selected card
*/
private Card selectCardFromDeckRandomly()
String rank = selectRanomCardRank();
String suit = selectRandomCardSuit();
return (new Card(rank, suit));
/*
* Randomly select a card rank
* @ return the selected card rank
*/
private String selectRanomCardRank()
String cardRank;
int r = 1 + rgen.nextInt(13);
switch (r)
case 1:
cardRank = "Ace";
break;
case 2:
cardRank = "2";
break;
case 3:
cardRank = "3";
break;
case 4:
cardRank = "4";
break;
case 5:
cardRank = "5";
break;
case 6:
cardRank = "6";
break;
case 7:
cardRank = "7";
break;
case 8:
cardRank = "8";
break;
case 9:
cardRank = "9";
break;
case 10:
cardRank = "10";
break;
case 11:
cardRank = "Jack";
break;
case 12:
cardRank = "Queen";
break;
default:
cardRank = "King";
break;
return cardRank;
/*
* Randomly select a card suit
* @ return The selected card suit
*/
private String selectRandomCardSuit()
String cardSuit;
int s = 1 + rgen.nextInt(4);
switch (s)
case 1:
cardSuit = "Clubs";
break;
case 2:
cardSuit = "Diamonds";
break;
case 3:
cardSuit = "Hearts";
break;
default:
cardSuit = "Spades";
break;
return cardSuit;
class Card:
/*
* Card.java
* -------------
* Creates a playing card of certain Rank and Suit.
* This class is part of exercises 1 solution.
* This program is intended as a practice on programming methodologies.
*/
public class Card
/* Instance variables */
private String rank; /* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
private String suit; /* The card suit : Clubs, Diamonds, Hearts, Spades */
/* Constructor */
/**
* Creates an instance object of class Card with the specified rank and suit
* @param rnk The rank of the card
* @param sut The suit of the card
*/
public Card(String rnk, String sut)
rank = rnk;
suit = sut;
public String toString()
return (rank + " of " + suit);
Note: still not in a position to use arrays.
Evaluation points:
- Encapsulation
- Top-Down design (refactoring)
- Readability and complexity reduction
- Object oriented
- Documentation
java beginner random playing-cards
still new to java and i am trying to improve my understanding and use of OOP concepts.here i am supposed to print the name of a randomly selected card from a deck of 52 cards.
Program:
/*
* RandomCard.java
* -------------------
* Displays the name of a card randomly chosen from a complete deck of 52
* playing cards.
*/
import java.util.Random;
public class RandomCard
/* Instance Variables */
private Random rgen = new Random(); /* random generator */
/**
* Display the name of a randomly selected card from a deck of 52 cards
*/
public static void main(String args)
System.out.println("This program displays a randomly chosen card");
Card randomCard = selectCardFromDeckRandomly();
System.out.println(randomCard);
/*
* Randomly select a card from a card deck
* @ return The selected card
*/
private Card selectCardFromDeckRandomly()
String rank = selectRanomCardRank();
String suit = selectRandomCardSuit();
return (new Card(rank, suit));
/*
* Randomly select a card rank
* @ return the selected card rank
*/
private String selectRanomCardRank()
String cardRank;
int r = 1 + rgen.nextInt(13);
switch (r)
case 1:
cardRank = "Ace";
break;
case 2:
cardRank = "2";
break;
case 3:
cardRank = "3";
break;
case 4:
cardRank = "4";
break;
case 5:
cardRank = "5";
break;
case 6:
cardRank = "6";
break;
case 7:
cardRank = "7";
break;
case 8:
cardRank = "8";
break;
case 9:
cardRank = "9";
break;
case 10:
cardRank = "10";
break;
case 11:
cardRank = "Jack";
break;
case 12:
cardRank = "Queen";
break;
default:
cardRank = "King";
break;
return cardRank;
/*
* Randomly select a card suit
* @ return The selected card suit
*/
private String selectRandomCardSuit()
String cardSuit;
int s = 1 + rgen.nextInt(4);
switch (s)
case 1:
cardSuit = "Clubs";
break;
case 2:
cardSuit = "Diamonds";
break;
case 3:
cardSuit = "Hearts";
break;
default:
cardSuit = "Spades";
break;
return cardSuit;
class Card:
/*
* Card.java
* -------------
* Creates a playing card of certain Rank and Suit.
* This class is part of exercises 1 solution.
* This program is intended as a practice on programming methodologies.
*/
public class Card
/* Instance variables */
private String rank; /* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
private String suit; /* The card suit : Clubs, Diamonds, Hearts, Spades */
/* Constructor */
/**
* Creates an instance object of class Card with the specified rank and suit
* @param rnk The rank of the card
* @param sut The suit of the card
*/
public Card(String rnk, String sut)
rank = rnk;
suit = sut;
public String toString()
return (rank + " of " + suit);
Note: still not in a position to use arrays.
Evaluation points:
- Encapsulation
- Top-Down design (refactoring)
- Readability and complexity reduction
- Object oriented
- Documentation
java beginner random playing-cards
edited Jul 18 at 20:37
200_success
123k14143399
123k14143399
asked Jul 18 at 18:06
ahmed dahy
264
264
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21
add a comment |Â
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
2
down vote
accepted
You did many things right. Great! =)
Readability and complexity reduction
Your
private String rank;
is nice and readable, but what is a rnk in
public Card(String rnk, String sut)
?
I would not suggest to compromise on the readability of publicly visible variable names. (And I would neither suggest doing so on any other variable of any access level.)
If there's no good alternative name for parameter names, use this to differentiate the readable (and equal) variable and parameter names:
public Card(String rank, String suit)
this.rank = rank;
this.suit = suit;
A common alternative solution is to add an underscore character to the variable name like so:
private String _rank;
private String _suit;
Speaking of those two variables, it might make sense to make them final. After all, the suit of a card should never be changed. This reduces complexity, because you can only ever set the value once.
Top-Down design (refactoring)
a randomly selected card from a deck of 52 cards.
You are not doing that at all. You pick some random rank and some other random suit and construct a card from that. The concept of a "deck" doesn't exist at all in your code. You merely have it implicitely defined just by how your random number generation works.
From top-down design code, I'd expect to see some object representing the deck of cards and some mechanism to get a random card from it. E.g.
Deck cardDeck = new Deck();
Card randomCard = cardDeck.getRandomCard();
Documentation
The public documentation of the constructor parameters
* @param rnk The rank of the card
* @param sut The suit of the card
is not adding any new information, while the hidden comments of the variables do:
/* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
/* The card suit : Clubs, Diamonds, Hearts, Spades */
I wish I could see those when being clueless about how to invoke the constructor.
My personal opinion is that comments like
/* Instance variables */
and
/* Constructor */
do not add any value. If you want them or if you are forced to add them, you should be consistent though and also add
/* Public methods*/
to your classes.
Object oriented
rank and suit should be Enum Types. One part of object oriented philosophy is to create types in a way that they are helpful. A string is a string, but a Suit is not a Rank. Among many others, this has the benefit of preventing mistakes like the following from even compiling:
private Card selectCardFromDeckRandomly()
String rank = selectRandomCardSuit();
String suit = selectRanomCardRank();
return (new Card(rank, suit));
Note: still not in a position to use arrays.
You actually are in exactly that position.
In you case, limiting your code to not use arrays is what reduces your ability to improve it the most.
The use of enums leads to the use of their values() method (which returns an array), which allows you to get rid of the switches, resulting in more generic code.
Oh, and btw. isn't String args an array?
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
2
down vote
accepted
You did many things right. Great! =)
Readability and complexity reduction
Your
private String rank;
is nice and readable, but what is a rnk in
public Card(String rnk, String sut)
?
I would not suggest to compromise on the readability of publicly visible variable names. (And I would neither suggest doing so on any other variable of any access level.)
If there's no good alternative name for parameter names, use this to differentiate the readable (and equal) variable and parameter names:
public Card(String rank, String suit)
this.rank = rank;
this.suit = suit;
A common alternative solution is to add an underscore character to the variable name like so:
private String _rank;
private String _suit;
Speaking of those two variables, it might make sense to make them final. After all, the suit of a card should never be changed. This reduces complexity, because you can only ever set the value once.
Top-Down design (refactoring)
a randomly selected card from a deck of 52 cards.
You are not doing that at all. You pick some random rank and some other random suit and construct a card from that. The concept of a "deck" doesn't exist at all in your code. You merely have it implicitely defined just by how your random number generation works.
From top-down design code, I'd expect to see some object representing the deck of cards and some mechanism to get a random card from it. E.g.
Deck cardDeck = new Deck();
Card randomCard = cardDeck.getRandomCard();
Documentation
The public documentation of the constructor parameters
* @param rnk The rank of the card
* @param sut The suit of the card
is not adding any new information, while the hidden comments of the variables do:
/* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
/* The card suit : Clubs, Diamonds, Hearts, Spades */
I wish I could see those when being clueless about how to invoke the constructor.
My personal opinion is that comments like
/* Instance variables */
and
/* Constructor */
do not add any value. If you want them or if you are forced to add them, you should be consistent though and also add
/* Public methods*/
to your classes.
Object oriented
rank and suit should be Enum Types. One part of object oriented philosophy is to create types in a way that they are helpful. A string is a string, but a Suit is not a Rank. Among many others, this has the benefit of preventing mistakes like the following from even compiling:
private Card selectCardFromDeckRandomly()
String rank = selectRandomCardSuit();
String suit = selectRanomCardRank();
return (new Card(rank, suit));
Note: still not in a position to use arrays.
You actually are in exactly that position.
In you case, limiting your code to not use arrays is what reduces your ability to improve it the most.
The use of enums leads to the use of their values() method (which returns an array), which allows you to get rid of the switches, resulting in more generic code.
Oh, and btw. isn't String args an array?
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
add a comment |Â
up vote
2
down vote
accepted
You did many things right. Great! =)
Readability and complexity reduction
Your
private String rank;
is nice and readable, but what is a rnk in
public Card(String rnk, String sut)
?
I would not suggest to compromise on the readability of publicly visible variable names. (And I would neither suggest doing so on any other variable of any access level.)
If there's no good alternative name for parameter names, use this to differentiate the readable (and equal) variable and parameter names:
public Card(String rank, String suit)
this.rank = rank;
this.suit = suit;
A common alternative solution is to add an underscore character to the variable name like so:
private String _rank;
private String _suit;
Speaking of those two variables, it might make sense to make them final. After all, the suit of a card should never be changed. This reduces complexity, because you can only ever set the value once.
Top-Down design (refactoring)
a randomly selected card from a deck of 52 cards.
You are not doing that at all. You pick some random rank and some other random suit and construct a card from that. The concept of a "deck" doesn't exist at all in your code. You merely have it implicitely defined just by how your random number generation works.
From top-down design code, I'd expect to see some object representing the deck of cards and some mechanism to get a random card from it. E.g.
Deck cardDeck = new Deck();
Card randomCard = cardDeck.getRandomCard();
Documentation
The public documentation of the constructor parameters
* @param rnk The rank of the card
* @param sut The suit of the card
is not adding any new information, while the hidden comments of the variables do:
/* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
/* The card suit : Clubs, Diamonds, Hearts, Spades */
I wish I could see those when being clueless about how to invoke the constructor.
My personal opinion is that comments like
/* Instance variables */
and
/* Constructor */
do not add any value. If you want them or if you are forced to add them, you should be consistent though and also add
/* Public methods*/
to your classes.
Object oriented
rank and suit should be Enum Types. One part of object oriented philosophy is to create types in a way that they are helpful. A string is a string, but a Suit is not a Rank. Among many others, this has the benefit of preventing mistakes like the following from even compiling:
private Card selectCardFromDeckRandomly()
String rank = selectRandomCardSuit();
String suit = selectRanomCardRank();
return (new Card(rank, suit));
Note: still not in a position to use arrays.
You actually are in exactly that position.
In you case, limiting your code to not use arrays is what reduces your ability to improve it the most.
The use of enums leads to the use of their values() method (which returns an array), which allows you to get rid of the switches, resulting in more generic code.
Oh, and btw. isn't String args an array?
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
add a comment |Â
up vote
2
down vote
accepted
up vote
2
down vote
accepted
You did many things right. Great! =)
Readability and complexity reduction
Your
private String rank;
is nice and readable, but what is a rnk in
public Card(String rnk, String sut)
?
I would not suggest to compromise on the readability of publicly visible variable names. (And I would neither suggest doing so on any other variable of any access level.)
If there's no good alternative name for parameter names, use this to differentiate the readable (and equal) variable and parameter names:
public Card(String rank, String suit)
this.rank = rank;
this.suit = suit;
A common alternative solution is to add an underscore character to the variable name like so:
private String _rank;
private String _suit;
Speaking of those two variables, it might make sense to make them final. After all, the suit of a card should never be changed. This reduces complexity, because you can only ever set the value once.
Top-Down design (refactoring)
a randomly selected card from a deck of 52 cards.
You are not doing that at all. You pick some random rank and some other random suit and construct a card from that. The concept of a "deck" doesn't exist at all in your code. You merely have it implicitely defined just by how your random number generation works.
From top-down design code, I'd expect to see some object representing the deck of cards and some mechanism to get a random card from it. E.g.
Deck cardDeck = new Deck();
Card randomCard = cardDeck.getRandomCard();
Documentation
The public documentation of the constructor parameters
* @param rnk The rank of the card
* @param sut The suit of the card
is not adding any new information, while the hidden comments of the variables do:
/* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
/* The card suit : Clubs, Diamonds, Hearts, Spades */
I wish I could see those when being clueless about how to invoke the constructor.
My personal opinion is that comments like
/* Instance variables */
and
/* Constructor */
do not add any value. If you want them or if you are forced to add them, you should be consistent though and also add
/* Public methods*/
to your classes.
Object oriented
rank and suit should be Enum Types. One part of object oriented philosophy is to create types in a way that they are helpful. A string is a string, but a Suit is not a Rank. Among many others, this has the benefit of preventing mistakes like the following from even compiling:
private Card selectCardFromDeckRandomly()
String rank = selectRandomCardSuit();
String suit = selectRanomCardRank();
return (new Card(rank, suit));
Note: still not in a position to use arrays.
You actually are in exactly that position.
In you case, limiting your code to not use arrays is what reduces your ability to improve it the most.
The use of enums leads to the use of their values() method (which returns an array), which allows you to get rid of the switches, resulting in more generic code.
Oh, and btw. isn't String args an array?
You did many things right. Great! =)
Readability and complexity reduction
Your
private String rank;
is nice and readable, but what is a rnk in
public Card(String rnk, String sut)
?
I would not suggest to compromise on the readability of publicly visible variable names. (And I would neither suggest doing so on any other variable of any access level.)
If there's no good alternative name for parameter names, use this to differentiate the readable (and equal) variable and parameter names:
public Card(String rank, String suit)
this.rank = rank;
this.suit = suit;
A common alternative solution is to add an underscore character to the variable name like so:
private String _rank;
private String _suit;
Speaking of those two variables, it might make sense to make them final. After all, the suit of a card should never be changed. This reduces complexity, because you can only ever set the value once.
Top-Down design (refactoring)
a randomly selected card from a deck of 52 cards.
You are not doing that at all. You pick some random rank and some other random suit and construct a card from that. The concept of a "deck" doesn't exist at all in your code. You merely have it implicitely defined just by how your random number generation works.
From top-down design code, I'd expect to see some object representing the deck of cards and some mechanism to get a random card from it. E.g.
Deck cardDeck = new Deck();
Card randomCard = cardDeck.getRandomCard();
Documentation
The public documentation of the constructor parameters
* @param rnk The rank of the card
* @param sut The suit of the card
is not adding any new information, while the hidden comments of the variables do:
/* The card rank : Ace,2,3,4,5,6,7,8,9,10,Jack,Queen,King */
/* The card suit : Clubs, Diamonds, Hearts, Spades */
I wish I could see those when being clueless about how to invoke the constructor.
My personal opinion is that comments like
/* Instance variables */
and
/* Constructor */
do not add any value. If you want them or if you are forced to add them, you should be consistent though and also add
/* Public methods*/
to your classes.
Object oriented
rank and suit should be Enum Types. One part of object oriented philosophy is to create types in a way that they are helpful. A string is a string, but a Suit is not a Rank. Among many others, this has the benefit of preventing mistakes like the following from even compiling:
private Card selectCardFromDeckRandomly()
String rank = selectRandomCardSuit();
String suit = selectRanomCardRank();
return (new Card(rank, suit));
Note: still not in a position to use arrays.
You actually are in exactly that position.
In you case, limiting your code to not use arrays is what reduces your ability to improve it the most.
The use of enums leads to the use of their values() method (which returns an array), which allows you to get rid of the switches, resulting in more generic code.
Oh, and btw. isn't String args an array?
answered Jul 18 at 21:48
I'll add comments tomorrow
2,010418
2,010418
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
add a comment |Â
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
I can not thank you enough. your answer is very valuable and rich.
â ahmed dahy
Jul 20 at 19:39
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f199765%2fchoose-random-card-from-a-deck%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
fixed indentation. which parentheses? you mean at return statement? i deliberately included those parentheses to make it clear what is returned. not sure if that is bad or just unnecessary. appreciate any advice on that.
â ahmed dahy
Jul 18 at 19:58
The parenthesis that you added in Rev 3.
â 200_success
Jul 18 at 20:36
Your code is not selecting randomly from a Deck. It can result in duplicate choices. Instead Create a deck of the pre-generated cards. Then select and remove a card from the deck. Code is broadly OK, but the #1 review criteria is always meeting the requirements. You'll need at least three Class, Card (present) Deck (missing) and a Dealer.
â Martin Spamer
Jul 19 at 18:21