二次元ArrayListを引数にして渡して処理したら、特定の一部の要素に変化が生じる

Question

お世話になっております。現在オセロゲームを作ろうと考えています。そこで盤面を、JButtonを敷き詰めることで表現しようと考えています、具体的には次のようなコードです。CellはJButtonを継承したもので、盤面内での座標をint[] distanceとして持っています。

＜ButtonPanel＞
	private ArrayList<ArrayList<Cell>> board = new ArrayList<ArrayList<Cell>>(8);
～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～
		for(int i = 0; i < 8; i++) {
			ArrayList<Cell> s =  new ArrayList<Cell>(8);
			for(int j = 0; j < 8; j++) {
				/* i = y座標
				 * j = x座標
				 */
				Cell c;
				if( j == 3 && i== 3 ||
					j == 4 && i== 4) {
					c = new Cell(j,i,-1);	//白
				}else if(j == 3 && i == 4 ||
						  j == 4 && i == 3) {
					c = new Cell(j,i,1);	//黒
				}else {
					c = new Cell(j,i,0);
				｝
				c.addActionListener(this);
				c.setActionCommand(j +","+ i);
				this.add(c);
				s.add(c);
			}
			board.add(s);
		}

そしてこの後、盤面であるboardを、CheckCellsというクラス(置ける石の位置を探すためのクラス)に引数で送ります。

＜CheckCells＞
	private ArrayList<ArrayList<Cell>> board = new ArrayList<ArrayList<Cell>>(8);
～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～
	public CheckCells(int a,ArrayList<ArrayList<Cell>> b) {
　　　　//コンストラクタ
		this.board.addAll(b);
	}
～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～～
	public HashSet<int[]> checkOk(){
		//盤面(board)をもとに石を探す処理

この一連の流れではエラーなどは出ないのですが、checkOkで返ってくる結果がおかしいため、デバックで調べてみました。
その結果、生成時(ButtonPanel)では何の以上もないのですが、CheckCellsで処理に使われているboardでは、特定のCellが持つdistanceに変化が生じるのです。
具体的には、boardの３行目4番（board.get(3).get(4)）のdistanceが[5,2]、4行目3番が[2,5]となったり
3行目4番が[3,5]、4行目3番[5,3]となったりする具合です。ほかの要素はまったく変わっていませんでした。

大変長くなってしまったのですが、簡単に言うと、生成時では問題なかったはずの二次元リストboardを、引く数として渡し処理で使ったとたんリスト内の特定の部分の要素が変化してしまう、というのが今回の問題点です。boardに対しては一貫してgetなどしか使っていないため、どこかで要素の追加や置き換え、削除を行ったりはしていません。（少なくとも自分が書いたコード内では）なぜこんな現象が起こるのでしょうか？

説明が下手で大変申し訳ありません。クラスが多めで処理も長いのですべてのクラスのコードは記載しませんが、不明な点があれば追記していきたいと思います。ご助力お願いいたします。

Answer 1

CellはJButtonを継承したもので、盤面内での座標をint[] distanceとして持っています。

これがどうしても引っ掛かります。
例えboardから一貫してgetしか使用していなくても、getしたcellから座標を配列の参照でそのまま取得し、その数値を変化させた場合、そのcellすべての座標が影響を受けることになります。

Comments

[tarutarupop]

つまり、boardにあるCell Aの持つdistance(int[])をgetして、int[]の変数 Bに入れたとします。そのBに対して、例えばB++などの操作をした場合、Cell Aの持つdistanceの情報が変わってしまうということでしょうか？

[swordone]

そうです。getの段階で配列をコピーしていない限り、Cell Aの持つ配列、Bが指す配列は全く同じものです。Bに対して変更した場合、同じ配列を参照しているAにとっての配列も当然に変化します。

[tarutarupop]

参照型変数ということを失念していました...checkOk内での処理でこれをやってしまっているので、それが原因だと思われます。ありがとうごさいます

[swordone]

そもそもCell(石)が座標を持つ必要があるのかというのも疑問だが…

Answer 2

質問文で引用している部分に関しては問題はないです。
デバック実行とのことなので、ウォッチ式を設定していませんか？
ブレイクポイントしたタイミングで式が評価されて結果が変わります。

---

質問文のコードではc = new Cell(j,i,-1);とiとjを逆に渡しているので、コンストラクタでそのまま設定していると仮定すると

board.get(3).get(4)　このコードで取得できるのは、4行目の3番のマスの値では？

printデバックとして、Cell に以下のようなtoStringメソッドを追加してみてはどうでしょうか？

@SuppressWarnings("serial")
class Cell extends JButton {
	// 質問の本題と関係ないので、distanceとpieceをpublicで定義しましたが、本来はGetter/Setterを定義する形の方が良いです。
	public final int[] distance;
	public int piece;// 駒

	public Cell(int j, int i, int j2) {
		this.distance = new int[] { j, i };
		this.piece = j2;
	}

	@Override
	public String toString() {
		return Arrays.toString(distance) + ":" + String.valueOf(this.piece);
	}
}

ButtonPanelクラスに以下のコードを追加

	public void dump(ArrayList<ArrayList<Cell>> board) {
		for (ArrayList<Cell> r : board) {
			System.out.println(Arrays.toString(r.toArray(new Cell[0])));
		}
	}

気になる点
0. CheckCellsクラスのcheckOkメソッドはHashSet<int[]>で値を返していますが、HashSetはセットの繰り返し順序について保証しません。

1. CellはJButtonを継承したもので、盤面内での座標をint[] distanceとして持っています。

JButtonは表示とボタンアクションの通知だけにして、JPanelと盤面の情報を管理するクラス（Boardクラス）を作成して、JPanelのイベントでアクションコマンドの引数を元に紐付けする形にもできるかと。

1. オセロなら盤面が固定(8*8)なため、生成時に要素が固定な多重配列(ジャグ配列)で管理するのも一つの手かと。これなら生成し直さない限り、要素数の変動は原理上起こりえません。

2. CheckCellsという置ける石の位置を探すためのクラスで盤面情報を管理するクラスにひとまとめにしたほうがという2重管理を防げるかと。

import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
import java.util.Arrays;

public class A123942 {

	public static void main(String[] args) {
		Board b = new Board();
		System.out.println(b);
	}
}

class Board {
	private final int[][] board = new int[8][8];
	private final PropertyChangeSupport changes = new PropertyChangeSupport(this);

	Board() {
		board[3][3] = -1;
		board[4][4] = -1;
		board[3][4] = 1;
		board[4][3] = 1;

		// もしくはEnumを使うなら変数:bordの型を変更して以下の初期化でも
		// for(Piece[] x: board){
		// Arrays.fill(x, Piece.None);
		// }
		// board[3][3] = Piece.White;
		// board[4][4] = Piece.White;
		// board[3][4] = Piece.Black;
		// board[4][3] = Piece.Black;
	}

	public void addPropertyChangeListener(PropertyChangeListener listener) {
		changes.addPropertyChangeListener(listener);
	}

	public void removePropertyChangeListener(PropertyChangeListener listener) {
		changes.removePropertyChangeListener(listener);
	}

	public void setPiece(int x, int y, int s) {
		int oldValue = board[x][y];
		board[x][y] = s;
		// 変更イベントを発火
		this.changes.firePropertyChange("setPiece", oldValue, s);
	}

	@Override
	public String toString() {
		return Arrays.deepToString(board);
	}
}

enum Piece {
	Black("黒"), White("白"), None(" ");
	private final String label;

	Piece(String label) {
		this.label = label;
	}

	@Override
	public String toString() {
		return this.label;
	}
}

Comments

[tarutarupop]

すいません、説明不足の点があったようです。
・JButtonをGridlayoutで表示しているのですが、試したところButtonPanelでのような生成の仕方をするとboard.get(a).get(b)のaが行bが列になるようでした。自分の感覚としては座標は(x,y)の順で保持したかったため、生成時にiとjを逆にして、あえてdistanceが(x,y)の順に格納されるようにしています。

1>「石を置くことができる位置」は数学でいうとこの集合のような感覚で使いたかったのでSetで管理しようと思いました。(今後の処理に順序が関係ないかなと思ったので...)
2,3,4>確かに、Cellは処理しないので情報を持っている意味はありませんね...改善案をもとに少し見直してみます。具体的な指摘ありがとうございます。