Constructor call must be the first statement in a constructor using this

Question

Why can't I make second constructor using switch case statements that refer input to the first constructor? It shows error "Constructor call must be the first statement in a constructor using this". So it seems that I have to retype assignments from 1st constructor for every case statement in the second.

public class Card {
        public static final String CLUBS = "Clubs";
        public static final String DIAMONDS = "Diamonds";
        public static final String HEARTS = "Hearts";
        public static final String SPADES = "Spades";
        public static final int ACE = 1;
        public static final int JACK = 11;
        public static final int QUEEN = 12;
        public static final int KING = 13;

        public Card(int rank, String suit) {
            this.rank = rank;
            this.suit = suit;
        }

        public Card(String rank, String suit) {
            if (!isCorrectSuit(suit)) throw new IllegalArgumentException("incorrect suit");
            switch(rank) {
            case ACE: this(1, suit);
            case JACK: this(11, suit);
            case QUEEN: this(12, suit);
            case KING : this(13, suit);
            default: throw new IllegalArgumentException("incorrect rank");
            }
        }



        private boolean isCorrectSuit(String suit) {
            return (suit.equals(CLUBS) || suit.equals(DIAMONDS) || suit.equals(HEARTS) || suit.equals(SPADES));
        }

        private boolean isCorrectRank(int rank) {
            return rank == 1 || rank == 11 || rank == 12 || rank == 13;
        }

        private int rank;
        private String suit;    

    }

In your shoes, I'd consider to write `enum`s for **both** suit and rank of cards. — zlakad, Mar 15 '18 at 10:52

Andrew Tobilko · Accepted Answer · 2018-03-15T11:54:55.490

You are probably looking for a static factory method where this(...) can be replaced with new Card(...):

class Card {
    ...

    private Card(int rank, String suit) {
        this.rank = rank;
        this.suit = suit;
    }

    public static Card of(String rank, String suit) {
        if (!isCorrectSuit(suit)) {
            throw new IllegalArgumentException("incorrect suit");
        }

        final Card card;
        switch (Integer.valueOf(rank)) {
            case ACE:
                card = new Card(1, suit);
                break;
            case JACK:
                card = new Card(11, suit);
                break;
            case QUEEN:
                card = new Card(12, suit);
                break;
            case KING:
                card = new Card(13, suit);
                break;
            default: {
                throw new IllegalArgumentException("incorrect rank");
            }
        }
        return card;
    }
}

These four instances can be predefined, you needn't create them over again.

I wonder why you didn't apply the same approach to the rank value:

public Card(int rank, String suit) {
    if (!isCorrectSuit(suit)) {
        throw new IllegalArgumentException("incorrect suit");
    }

    if (!isCorrectRank(rank)) {
        throw new IllegalArgumentException("incorrect rank");
    }

    this.rank = rank;
    this.suit = suit;
}

public Card(String rank, String suit) {
    this(Integer.valueOf(rank), suit);
}

Note that you've got 2 public constructors, and only one of them has a kind of validation. I can create a new Card(2, "Unknown") and will get no exceptions.

Other options for consideration might be:

writing enums instead of primitive values
writing sets of values to replace the isCorrectX methods with Set#contains

A less "extreme" way to do this is to write a static method that simply converts `String rank` to an `int`, and invoke that in the existing constructor: `this(stringRankToInt(rank), suit)`. — Andy Turner, Mar 15 '18 at 10:51
@AndyTurner it seems `stringRankToInt` simply means `Integer.valueOf` — Andrew Tobilko, Mar 15 '18 at 11:07

score 0 · Answer 2 · answered Mar 15 '18 at 10:46

If you want invoke one constructor from another within the class you invoke it through this(args) key word with adequate arguments.

It's not fully clear which constructor you want to use in which, but I think this is more probable:

  public Card(int rank, String suit) {
        this.rank = rank;
        this.suit = suit;
    }

    public static Card create(String rank, String suit) {

        if (!isCorrectSuit(suit)) throw new IllegalArgumentException("incorrect suit");
        switch(rank) {
        case ACE: new Card(1, suit); break;
        case JACK: new Card(11, suit); break;
        case QUEEN: new Card(12, suit); break;
        case KING : new Card(13, suit); break;
        default: throw new IllegalArgumentException("incorrect rank");
        }
    }

you have a static method that lets you create an object of type Card.

score -1 · Answer 3 · answered Mar 15 '18 at 10:48

-1

just add a static routine that converts your String to an int

answered Mar 15 '18 at 10:48

Paul Janssens

622
3
9

Constructor call must be the first statement in a constructor using this

3 Answers3