Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Just another novice Java coder asking for approval

Tags:

java

So... I'm learning some Java recently and I want You to help me with my very first program, if You like. This is my code for program, which displays random card from the deck.

My question is following: is there any smarter way to make this program work?

/*
 * File: RandomCard.java
 * ----------------
 * This program display random card from the deck with it's own rank
 * (Ace, 2, 3, 4, 5, 6, 7, 8, 9, 10, Jack, Quenn, King) and suit (Clubs,
 * Diamonds, Hearts, Spades).
 */

import acm.program.*;
import acm.util.*;

public class RandomCard extends ConsoleProgram {
    /* Run the program */
    public void run() {
        println("This program displays a random card from the deck.");
        println("You random card is " + getRandomRank() + " "
                + getRandomSuit() + ".");       
    }

    /* Get random rank for the card. */
    private String getRandomRank() {
        int rank = rgen.nextInt(1, 13);
        switch (rank) {
            case 1: return "Ace";
            case 2: return "2";
            case 3: return "3";
            case 4: return "4";
            case 5: return "5";
            case 6: return "6";
            case 7: return "7";
            case 8: return "8";
            case 9: return "9";
            case 10: return "10";
            case 11: return "Jack";
            case 12: return "Queen";
            case 13: return "King";
            default: return null;
        }
    }

    /* Create random suit from within Clubs, Diamonds, Hearts and Spades. */
    private String getRandomSuit() {
        int suit = rgen.nextInt(0, 3);
        switch (suit) {
            case 0: return "Clubs";
            case 1: return "Diamonds";
            case 2: return "Hearts";
            case 3: return "Spades";
            default: return null;
        }
    }

    /* Create an instance variable for the random number generator */
    private RandomGenerator rgen = new RandomGenerator();
}
like image 420
Chris Avatar asked Aug 24 '26 17:08

Chris


2 Answers

A minor improvement: any time you have a switch statement like this:

 switch (rank) {
        case 1: return "Ace";
        case 2: return "2";
        case 3: return "3";
        case 4: return "4";
        case 5: return "5";
        case 6: return "6";
        case 7: return "7";
        case 8: return "8";
        case 9: return "9";
        case 10: return "10";
        case 11: return "Jack";
        case 12: return "Queen";
        case 13: return "King";
        default: return null;
    }

You can instead declare an array:

String[] ranks = { "Ace", "2", "3", "4", "5", "6", "7", "8", "9", "10", 
                   "Jack", "Queen", "King" };

And then:

if (rank < 1 || rank > 13) return null;
return ranks[rank - 1]; // arrays are zero-based
like image 128
Daniel Earwicker Avatar answered Aug 27 '26 00:08

Daniel Earwicker


A suggestion, instead of returning String for the Rank and Suite, I would have make them enum instead, that way they are typesafe.

public enum Rank { DEUCE, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN, JACK, QUEEN, KING, ACE }
public enum Suit { CLUBS, DIAMONDS, HEARTS, SPADES }

See here how it is done: http://download.oracle.com/javase/1.5.0/docs/guide/language/enums.html

like image 26
Rosdi Kasim Avatar answered Aug 26 '26 22:08

Rosdi Kasim



Donate For Us

If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!