2016-08-06 6 views
0

学習目的でポーカーシミュレータを作成していますが、Deckクラスを不変にする問題があります。配列を変更不能にする

public class Deck { 

    private final Card[] cards; 
    private int cardCount; 

    public Deck(@NotNull Card[] c) { 
      this.cards = Arrays.copyOf(c, c.length); 
      this.cardCount = c.length - 1; 
    } 

    public Deck shuffle() { 
      // shuffle logic 
      return new Deck(this.cards); 
    } 

    public Card pop() { 
      return this.cards[cardCount--]; 
    } 
    // other methods 
} 

DealerクラスはDeck参照してロジックを扱う道具を保持しています。それはpopが呼び出されるたびに更新されているcardCountを保持しているため

public class Dealer { 

    private final Deck deck; 

    public Dealer(@NotNull Deck d) { 
      this.deck = d; 
    } 

    public void deal(@NotNull Holder<Card> h) { 
      h.hold(deck.pop()); 
    } 
    // other methods 
} 

あなたがDeckを見ることができるように、完全に不変ではありません。注 - cardCountは外部には見えません。またCardクラスは不変です。

質問1:このデザインはどのように影響を受けますかDeck不変性?

質問2:将来の影響はどうなりますか?

質問3:Deckクラスを完全に不変にする価値はありますか?はいの場合は、どうですか?

ありがとうございました。

+1

あなたのクラスを可変にするカウンタだけでなく、配列が自然に変更可能であるため、リファレンスがfinalでも、カウンタも変更可能です。 –

+0

私は配列のために防御的なコピーを作成します。 –

+0

本当の問題は、あなたが突然変異操作として設計した 'pop'です。指定されたセマンティクスで 'pop'メソッドをサポートする必要がある限り、' Deck'は内部的な手抜きでは不変ではありません。 – user2357112

答えて

2

これは、不変性が役に立たない場合の1つです。これはその全体の目的であるので、あなたは可変共有状態を必要とするようです。Deck

これを変更できないようにするには、他のカードが1つ飛び出すたびに新しいDeckを作成する必要があります。しかし、あなたは(カードを一番上に置く前に)古いDeckを支配していないので、これは依然として一部のクライアントによって参照されている可能性があります。

現在の実装では、複数のスレッドで問題が発生する可能性があるため、クラスのスレッドを安全にすることを検討してください。カウンタを持つ配列の代わりにjava.util.Stackを使用すると、最初の試みとして有効です。

+0

私は今参照してください。同じ「デッキ」が異なる「ディーラー」によって使用される場合、私は困っているでしょう。しかし、私は「スタック」が私を助けてくれるのか見ていない。あるディーラーがカードをポップしてスタックを更新する場合、別のディーラーはそれをもう使用することができません。このための唯一の解決策は、すべてのディーラーがそれ自身の「デッキ」のコピーを持っていることを確認することです。 –

+0

@DennisGrinch yep ...各ディーラーだけでなく、デッキは変更可能でカードを削除するだけなので、新しいゲームを開始するたびに、同じディーラーを持っていても新しいデッキインスタンスが必要になります –

1

最大の問題は、Deckクラスで突然変異のメソッドがあることです。あなたはゲーム中にあなたがデッキに持っているカードを突然変異させることができる必要がありますが、実際にはそれのために実際のデッキオブジェクトを変更する必要はありません。あなたが不変のカードクラスと不変のデッキクラスを持っているとします。デッキは完全なカード補充で初期化されます。あなたがゲームをする必要があるときは、ディーラーが対話、変更などできるカードのリストを尋ねるので、Deckクラスは実際には内部リストのコピーを渡すだけです。そうすれば、ディーラーは内部リストを変更しておらず、要求されたときにデッキは新しいゲームの準備ができており、コピーされたリストには内部デッキクラスと同じカードインスタンスへの参照が保持されていても問題ありません。 。何の状態が共有されていないとデッキは、もはや変更可能であることから

public final class Card { 

    public enum Suit { HEART, SPADE, CLUB, DIAMOND } 
    public enum Value { ACE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN, JACK, QUEEN, KING } 

    private final Suit suit; 
    private final Value value; 

    public Card(Suit suit, Value value) { 
     this.suit = suit; 
     this.value = value; 
    } 

    // ... rest omitted 
} 

public final class Deck { 

    private final List<Card> cards = new ArrayList<>(); 

    public Deck() { 
     for (Card.Suit suit : Card.Suit.values()) { 
      for (Card.Value value : Card.Value.values()) { 
       cards.add(new Card(suit, value)); 
      } 
     } 
    } 

    // Get a new list of cards (though the actual card instances are the same 
    // that's ok since they are immutable) 
    public List<Card> getCards() { 
     return new ArrayList<>(cards); 
    } 
} 

はまた、スレッドの安全性は、もはや心配はありません。ディーラー用のカードのリストを使用するもう一つの利点は、Collectionsクラスを使用してソートされていること(カード実装のものがあれば)、シャッフルなどです。

編集:あなたのコメントを読んだ後に追加:

あなたはデッキには状態を保持したい場合、あなたはそれが不変であることはできません。これは、さまざまな方法で実装できます。ほとんどの場合、ArrayListは配列に比べてかなり大きなデータ構造ですが、その使用はLinkedListよりパフォーマンスが優れており、安全性と使いやすさの点で優れています。非常に高いレベルのパフォーマンスが必要な場合は、プリミティブ配列(おそらく)でうまくいくことができますが、Collections APIは非常に最適化されており、JVMはコードを最適化する上で非常に優れています。これまでの私の専門的な経験では、単にパフォーマンスのためにリストの上で配列を使用するケースにはまだ踏み込んでいません。一般的に私はそれが時期尚早な最適化のケースであると言います(それに注意してください)。 ArrayListの上にStackを使用するのは抽象的には意味がありますが、Listの組み込みシャッフルメソッドの便利さがCollectionをオーバーライドすると主張します。

次のように上記と同じカードのオブジェクトを使用して、変更可能なデッキは(リストに)になります:

public class EmptyDeckException extends RuntimeException { 

    static final long serialVersionUID = 0L; 

    public EmptyDeckException(String message) { 
     super(message); 
    } 
} 

public class Deck { 

    private final List<Card> cards = new ArrayList<>(); 

    public Deck() { 
     for (Card.Suit suit : Card.Suit.values()) { 
      for (Card.Value value : Card.Value.values()) { 
       cards.add(new Card(suit, value)); 
      } 
     } 
    } 

    public void shuffle() { 
     Collections.shuffle(cards); 
    } 

    public Card pop() { 
     if (!cards.isEmpty()) { 
      return cards.remove(cards.size() - 1); 
     } 
     throw new EmptyDeckException("The deck is empty"); 
    } 
} 

とスタックが上主張されている場合、それはこのようなことができます:

+0

入力用のThnksですが、 'ArrayList'はこのための膨大なデータ構造です。カードを保持するデータ構造として 'Deck'を実装したいだけで、' pop'と 'shuffle'メソッドしかありません。 –

+0

これは問題ありませんが、あなたのデッキに現在の状態を保持させたい場合、それは不変であるとみなすことはできません。また、なぜそれが膨らんだと言うのですか?それは他の方法の数のためか、それは演奏者ではないと思うからですか? –

+0

'ArrayList'は要素を効率的に格納、追加、削除するためのものです。しかし私の 'デッキ'はすべての機能を必要としません。そのために 'Stack'を使うのが良いですが、それは正しいデータ構造ではなく、' push'と 'pop'要素を引き起こします。一方、プレイポーカーを開始すると、カードのデッキがあり、カードをシャッフルし、上から1つずつ取ります。 –

1

私の意見では、変更可能性は2つのことに依存します。オブジェクトが不変である場合に必要なインスタンス数と不変なコピーが再利用されるかどうか。

最初は簡単ですが、デッキのすべての順列を作成する必要はありません。必要なときに新しいものを作成するだけで、不変にしないようにすることはできません。

ここで私は問題が再利用可能であることを見始めます。あなたはデッキを不変にしますが、不変性が新しいオブジェクトを提供して古いものを捨て去るだけであれば、オブジェクトをその場で突然変異させ、不要なオブジェクトを作成するのはもっと論理的です。

しかし、私は不変のカードを確実に使用しています。それらのうちの54個だけが存在し、それらは間違いなく再利用することができる。だから私はどうなるのかは、中にそれらのカードのキャッシュを保存しているListまたはSet不変:

private static final List<Card> CARDS = Collections.unmodifiableList(new ArrayList<Card>() {{ 
    //store all your cards, can be done without double-brace initialiation 
}}); 

そこから(Deck内であることの文脈では)、私たちは多くのメモリの原価計算せずに、このListのコピーを作成することができますJavaは "参照値渡し"であるため、Cardのインスタンスは同じになります。

//Note: The left hand side is purposefully LinkedList and not List, you'll see why 
private final LinkedList<Card> shuffledDeck = new LinkedList<>(); 

public Deck() { 
    this.shuffle(); 
} 

public Deck shuffle() { 
    this.shuffledDeck.clear(); //wipe out any remaining cards 
    this.shuffledDeck.addAll(CARDS); //add the whole deck 
    Collections.shuffle(this.shuffledDeck); //Finally, randomize 
    return this; 
} 

public Card pop() { 
    return this.shuffledDeck.pop(); //Thanks Deque, which LinkedList implements 
} 

これはあなたの数多くのDeckオブジェクトの原価計算ではないながら、Card不変を作るから保存されたメモリであなたを残します。

関連する問題