3

同期メソッドに関するいくつかの記事 (Oracle を含む) を読みましたが、それを正しく理解しているかどうかわかりません。

次のコードがあります。

public class Player extends javax.swing.JLabel implements Runnable{
    private static int off2[] = {-1, 0, 1, 0}, off1[] = {0, 1, 0, -1};
    private static final Map dir = new java.util.HashMap<Integer, Integer>();
    static{
        dir.put(KeyEvent.VK_UP, 0);
        dir.put(KeyEvent.VK_RIGHT, 1);
        dir.put(KeyEvent.VK_DOWN, 2);
        dir.put(KeyEvent.VK_LEFT, 3);
    }
    private boolean moving[] = new boolean[4];

    @Override
    public void run(){
        while(true){
            for(Object i : dir.values()){
                if(isPressed((Integer)i)) setLocation(getX() + off1[(Integer)i], getY() + off2[(Integer)i]);
            }
            try{
                Thread.sleep(10);
            }catch(java.lang.InterruptedException e){
                System.err.println("Interrupted Exception: " + e.getMessage());
            }
        }
    }
    public void start(){
        (new Thread(this)).start();
    }

    private synchronized boolean isPressed(Integer i){
        if(moving[i]) return true;
        else return false;
    }

    public synchronized void setPressed(KeyEvent evt) {
        if(dir.containsKey(evt.getKeyCode()))moving[(Integer)dir.get(evt.getKeyCode())] = true;
    }
    public synchronized void setReleased(KeyEvent evt){
        if(dir.containsKey(evt.getKeyCode()))moving[(Integer)dir.get(evt.getKeyCode())] = false;
    }
}

今のところ、それが行うのは移動だけです。私の理解では、メイン フォームのキー リスナーが keyPressed および Released イベントを登録すると、setPressed および setReleased がメイン スレッドから呼び出されます。このコードは合理的ですか？同期キーワードの適切な使用ですか? (コードはそれがなくても動作しますが、ある方が良いのではないでしょうか?)

4

2 に答える 2

1

同期の使用は正しいですが、現在の実装では、配列への並列アクセスによって不整合が発生しないため、必要ではないと思います。

ただし、コードには別のスレッドの問題があると思います。別のスレッドから Swing (=call setLocation()) とやり取りすることは想定されていません。http://docs.oracle.com/javase/1.4.2/docs/api/javax/swing/SwingUtilities.html#invokeLater( java.lang.Runnable) . したがって、更新コードを Runnable にラップする必要があります。

private boolean moving[] = new boolean[4];
Runnable dirUpdate = new Runnable() {
  for(Object i : dir.values()){
    if(isPressed((Integer)i)) setLocation(getX() + off1[(Integer)i], getY() + off2[(Integer)i]);
  }
};

スレッドからの呼び出しは次のようになります。

SwingUtils.invokeLater(dirUpdate);

この場合、dirUpdate はイベント処理スレッドから呼び出されるため、もはや同期する必要はないことに注意してください。

マップされていないキーでの例外を回避するために、 isPressed() で null をチェックすることができます。

動きのより単純な表現は、キー イベントによって -1、1、または 0 に設定される dx および dy 変数を持つことです。

于 2012-05-25T19:00:04.167 に答える
1

Swing側についてはよくわかりませんが、一般的に言えば、複数のスレッドからアクセスされる可能性のある共有データ（あなたの場合はmoving[]）を保護するために同期が必要です。

この場合、moving[] は setXxx メソッド (メイン スレッド) による書き込み時または開始したスレッドによる読み取り時にアクセスされる可能性があるため、同期する必要があります。

Java 5 以降、java.concurrent パッケージの機能ははるかに優れています。Lock を使用して moving[] を保護するか、AtomicBoolean を使用することを検討することをお勧めします。

いくつかの「スタイルの問題」:

  1. 「生の型」を使用しないでください -- Map よりも Map<Integer, Integer> を優先してください (これにより、setXxxx() メソッドでチェックされていない -ugly- キャストからも解放されます)

  2. 1 行の if を避ける -- コードを読みにくくする :)

Lock を使用することにした場合 (この場合は同期に対して大きな利点ではありません)、 isPressed() は次のようになります。

// ...
private Lock movingLock = new ReentrantLock();

private  boolean isPressed(Integer i){
  try {
    movingLock.acquire();
    return moving[i];
  } finally
    movingLock.release();
  }        
}

Lock.acquire() および release() への呼び出しを使用して setXxx メソッドの moving[] への割り当てを「ラップ」する必要があります。

于 2012-05-25T19:08:27.890 に答える