2011-02-02 9 views
6

私は、トレーニング目的のために、Javaでシンプルに同期化されたStackオブジェクトを作成しました。ここ は私がやったことです:ここではSynchronizedStackクラスを正しく作成する方法は?

public class SynchronizedStack { 
    private ArrayDeque<Integer> stack; 

    public SynchronizedStack(){ 
     this.stack = new ArrayDeque<Integer>();  
    } 

    public synchronized Integer pop(){ 
     return this.stack.pop(); 
    } 

    public synchronized int forcePop(){ 
     while(isEmpty()){ 
      System.out.println(" Stack is empty"); 
      try { 
       wait(); 
      } catch (InterruptedException e) { 
       e.printStackTrace(); 
      } 
     } 
     return this.stack.pop(); 
    } 

    public synchronized void push(int i){ 
     this.stack.push(i); 
     notifyAll(); 
    } 

    public boolean isEmpty(){ 
     return this.stack.isEmpty(); 
    } 

    public synchronized void pushAll(int[] d){ 
     for(int i = 0; i < d.length; i++){ 
      this.stack.push(i); 
     } 
     notifyAll(); 
    } 

    public synchronized String toString(){ 
     String s = "["; 
     Iterator<Integer> it = this.stack.iterator(); 
     while(it.hasNext()){ 
      s += it.next() + ", "; 
     } 
     s += "]"; 
     return s; 
    } 
} 

は私の質問です:

  • はそれがOK isEmtpy()メソッドを同期させることはありませんか?なぜなら、別のスレッドが同時にスタックを修正していても、それはまだ一貫性のある結果を返すからです(初期または最終ではないisEmpty状態になる操作はありません)。それとも、同期化されたオブジェクトのすべてのメソッドを同期させるのが良い設計ですか?

  • 私はforcePop()メソッドが嫌いです。要素をポップする前に項目がスタックにプッシュされるまで待つことができたスレッドを作成したかったので、スレッドのrun()メソッドでwait()のループを実行するのが最善の選択肢だと思っていましたが、それはIllegalMonitorStatExceptionを投げるからです。このようなことをする正しい方法は何ですか?

  • 他のコメント/ご提案ですか?

ありがとうございます!

+1

メソッドをロックしたり、オブジェクトをロックしたりしないでください。 –

+0

スタックは、既に同期されているベクトルを拡張します。このトレーニングの練習には、コレクションの選択肢が異なる方がよいでしょう。 –

+0

Don Roby:axtavtが指摘しているように、ArrayDequeを使用しています。 – nbarraille

答えて

7
  • Stack自体がすでに同期しているので、それは(あなたが非同期スタックの実装をしたい場合ArrayDequeを使用)

  • それは事実とは別に(OKではありませんもう一度同期を適用しても意味がありません。前の点から)、同期の欠如がメモリの可視性の影響を引き起こす可能性があるためです。

  • forcePop()はかなり良いです。割り込み可能なブロッキング方法の契約に従うことなく、それをキャッチせずにInterruptedExceptionを渡す必要があります。 Thread.interrupt()を呼び出してforcePop()コールでブロックされたスレッドを中断することができます。

+0

は私がスタック/ ArrayDequeの実装に依存することなく、これをコード化されていたと仮定し、ちょうど私は支離滅裂のisEmpty()の結果を引き起こす可能性が全く推移状態が存在しないと確信している方法で、アレイと。それを同期させないほうが速いのですか、とにかく同期する方が安全でしょうか? – nbarraille

+0

とにかくそれを同期 - 問題は、一部の同期せずに、あなたはのisEmpty()の呼び出しで別のスレッドからの更新を参照してください-never-かもしれない、ということです。 – Sbodd

0

isEmpty()を同期させないという唯一の問題は、あなたが何が起きているのかわからないことです。あなたの推論は、妥当であるが、根底にあるStackも妥当な方法で行動していると仮定している。これはおそらくこの場合ですが、一般的にそれに頼ることはできません。

質問の2番目の部分は、すべての可能な戦略の完全な実装については、thisを参照してください。

もう1つの提案:アプリケーションのいくつかの部分(または複数のアプリケーション)で再利用される可能性があるクラスを作成する場合は、​​メソッドを使用しないでください。代わりに、次の操作を行います。

public class Whatever { 
    private Object lock = new Object(); 

    public void doSomething() { 
    synchronized(lock) { 
     ... 
    } 
    } 
} 

この理由は、あなたのクラスのユーザーがWhateverインスタンスかに同期したい場合は、あなたが本当に知らないということです。もしそうであれば、クラスそのものの操作に干渉するかもしれません。あなたは自分のプライベートロックを持っているので、誰も干渉することはできません。

0

stack.isEmpty()は同期が必要ないと仮定しても、制御できないクラスの実装の詳細に頼っているとします。 Stackのjavadocsは、クラスがスレッドセーフでないことを示しているので、すべてのアクセスを同期する必要があります。

0

あなたはちょっとしたイディオムを混ぜていると思います。あなたは順番に​​あるjava.util.Vector、によって裏打ちされjava.util.Stack、とあなたSynchronizedStackを支持しています。私は別のクラスのwait()notify() behaivorをカプセル化する必要があると思います。

関連する問題