質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

88.04%

オセロプログラムの添削をお願いします

解決済

回答 3

投稿 編集

  • 評価
  • クリップ 3
  • VIEW 5,170

score 27

プログラミングの基礎ばかりを学習し、実践経験が全くない者です。
そこでこの度、実際にオセロのプログラムコードをJavaScriptで書いてみました。

しかし、このように一つの形にしたプログラミングは今回が初めてで、
普段からプログラミングに慣れ親しんでおられる方には、
非常に汚いコードに見えることと思います。

「こういう書き方はまずい」、「自分だったらこう書く」といったような
添削をしていただけると、今後プログラミングをしていく上で非常に助かります。

長いプログラムコードではありますが、何卒よろしくお願いいたします。

ソースコード

<script type="text/javascript">
<!-- 
    // 黒と白の画像を変数に格納
    var black = 'url("http://i.imgur.com/CLEpdaJ.png")';
    var white = 'url("http://i.imgur.com/VZgXNBM.png")';
    
    var turn = 1; // 1が黒、2が白
    
    var turnflg = false;
    var endflg = false;
    
    /* 10×10のフィールドを作る */
    var field = [
        [9, 9, 9, 9, 9, 9, 9, 9, 9, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
        [9, 9, 9, 9, 9, 9, 9, 9, 9, 9]
    ]; 
    
    /* ------------------------------------------------------- */
    
    function showBoard() {
        var element = document.getElementById("othello");
        
        // 盤面を描写する処理
        for(var y = 1; y <= 8; y++){
            for(var x = 1; x <= 8; x++) {
                field[y][x] = document.createElement("div");
                field[y][x].id = "p" + y + x;
                
                field[y][x].style.border = "1px solid black";
                field[y][x].style.cssFloat = "left";
                field[y][x].style.height = "60px";
                field[y][x].style.width = "60px";
                field[y][x].style.background = "green";
                field[y][x].style.cursor = "pointer";
                
                field[y][x].addEventListener("click", clickEvent, false);
                
                // 8ピース毎に改行する
                if((x % 8) == 1){
                    field[y][x].style.clear = "both";
                }
                element.appendChild(field[y][x]);
            }
        }
        
        // 黒と白の初期配置
        field[4][5].style.backgroundImage = black;
        field[5][4].style.backgroundImage = black;
        field[4][4].style.backgroundImage = white;
        field[5][5].style.backgroundImage = white;
    }
    
    // 石を返すことができるかどうかを判別する
    function checkAble(y, x){
        
        var pos_y = [-1, -1, 0, 1, 1, 1, 0, -1];
        var pos_x = [0, 1, 1, 1, 0, -1, -1, -1];
        
        // 選択した箇所に石が置かれていない場合のみ処理する
        if(field[y][x].style.backgroundImage == "none"){
            var stone;
            turn == 1 ? stone = black : stone = white
            
            // 選択した箇所の周囲8箇所に石が置けるかどうか判別する
            for(var pos = 0; pos < 8; pos++){
                var count = 0;
                var n = y + pos_y[pos];
                var m = x + pos_x[pos];
                
                // 周囲8箇所が壁、自分の石、空白の場合、以降の処理を行わない
                if(field[n][m] == 9 || field[n][m].style.backgroundImage == stone
                    || field[n][m].style.backgroundImage == "none"){
                    continue;
                }
                
                // 周囲8箇所の延長線上に相手の石がある間繰り返す
                while(field[n][m].style.backgroundImage != stone){
                    n += pos_y[pos];
                    m += pos_x[pos];
                    
                    count++;
                    
                    // 相手の石を挟んで自分の石がある場合、間の相手の石をひっくり返す
                    if(field[n][m] != 9 && field[n][m].style.backgroundImage == stone){
                        while(count >= 0){
                            n -= pos_y[pos];
                            m -= pos_x[pos];
                        
                            field[n][m].style.backgroundImage = stone;
                            count--;
                        }
                        
                        // 手番を交代する
                        turnflg = true;                        
                    }
                    
                    // 周囲8箇所の延長線上が壁、または相手の石を挟んで自分の石がない場合、
                    //何もせず処理を終了する
                    if(field[n][m] == 9 || field[n][m].style.backgroundImage == "none"){
                        break;
                    }
                }
            }
        }
    }
    
    function clickEvent() {
        var element = document.getElementById(this.id);
        var y, x;
        
        // 設定したdividから配列の要素を得る
        y = parseInt(element.id.substr(-2, 1));
        x = parseInt(element.id.substr(-1, 1));
        
        checkAble(y, x);
        
        // ゲームが終了するまでターンを入れ替える
        if(!endflg && turnflg){
            turn == 1 ? turn = 2 : turn = 1
            turnflg = false;
        }
    }
    
    onload = function() {
        showBoard();
    }
-->
</script>

// オセロプログラムを表示
<div id="othello"></div>

補足情報

仕様としては、一人で黒と白の石を持ち、相手の石を挟めるマス目にだけ置くことが可能で、
交互に打ち合うだけのものです。
  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

+3

元のコードを少し変更してみました。
  - グローバル変数を減らした。
  - 手番 (黒、白) を BLACK, WHITE の定数で指定するようにした。
  - field[][] には、石の状態 (integer) だけを保持するようにした。

TODO:
  ゲーム終了(勝敗がついた), パス(石を置ける場所がないので相手に手番を渡す) の処理をすること。
  石をひっくり返す時にアニメーションする。
  石を置ける場所をヘルプ表示する。

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="content-language" content="ja">
    <meta charset="UTF-8">
</head>
<html>
  <body>
    <div id="othello"></div>
    <script type="text/javascript" src="reversi.js"></script>
  </body>
</html>

var STONE_BLACK = 'url("http://i.imgur.com/CLEpdaJ.png")';
var STONE_WHITE = 'url("http://i.imgur.com/VZgXNBM.png")';
var BLACK = 1;
var WHITE = 2;

var turn = BLACK;    // 先手は黒

// 10×10のフィールドを作って、石の色を保持する。
// 0: 石がない, BLACK: 黒石, WHITE: 白石、9: 壁
var field = [
    [9, 9, 9, 9, 9, 9, 9, 9, 9, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 0, 0, 0, 0, 0, 0, 0, 0, 9],
    [9, 9, 9, 9, 9, 9, 9, 9, 9, 9]
];
// -------------------------------------------------------
// 盤面を表示する。
function showBoard() {
    var element = document.getElementById("othello");
    // 盤面を描写する処理
    for (var y = 1; y <= 8; y++) {
        for (var x = 1; x <= 8; x++) {
            var f = document.createElement("div");
            f.id = "p" + y + x;
            f.style.border = "1px solid black";
            f.style.cssFloat = "left";
            f.style.height = "60px";
            f.style.width = "60px";
            f.style.background = "green";
            f.style.cursor = "pointer";
            f.addEventListener("click", clickEvent, false);

            // 8ピース毎に改行する
            if ((x % 8) == 1) {
                f.style.clear = "both";
            }

            // 石を置く。
            if (field[y][x] !== 0) {
              var image = (field[y][x] == BLACK)? STONE_BLACK : STONE_WHITE;
              f.style.backgroundImage = image;
            }
            element.appendChild(f);
        }
    }
}

// player が(y、x)に石を置くことができるかどうかを判別する。
// 置ける場合は、盤面を更新する。
function playable(y, x, player) {
    var ans = false;
    var opponent = (player == BLACK) ? WHITE : BLACK;
    var delta_y = [-1, -1, 0, 1, 1, 1, 0, -1];
    var delta_x = [0, 1, 1, 1, 0, -1, -1, -1];

    // 選択した箇所に石が置かれていない場合のみ処理する
    if (field[y][x] === 0) {
        // 選択した箇所の周囲8箇所に石が置けるかどうか判別する
        for (var pos = 0; pos < 8; pos++) {
            var count = 0;
            var n = y + delta_y[pos];
            var m = x + delta_x[pos];

            // 周囲8箇所が壁、自分の石、空白の場合、以降の処理を行わない
            if (field[n][m] === 9 || field[n][m] === player || field[n][m] === 0) {
                continue;
            }
            // 周囲8箇所の延長線上に相手の石がある間繰り返す
            while (field[n][m] === opponent) {
                n += delta_y[pos];
                m += delta_x[pos];
                count++;

                // 相手の石を挟んで自分の石がある場合、間の相手の石をひっくり返す
                if (field[n][m] === player) {
                    ans = true;  // 石が置けた
                    while (count >= 0) {
                        n -= delta_y[pos];
                        m -= delta_x[pos];
                        var f = document.getElementById("p" + n + m);
                        f.style.backgroundImage = (player == BLACK)? STONE_BLACK : STONE_WHITE;
                        field[n][m] = player;
                        count--;
                    }
                }
            }
        }
    }
    return ans;
}

function clickEvent() {
    var element = document.getElementById(this.id);
    // 設定したdividから配列の要素を得る
    var y = parseInt(element.id.substr(-2, 1));
    var x = parseInt(element.id.substr(-1, 1));

    var check = playable(y, x, turn);
    if (check) {
        turn = (turn == BLACK)? WHITE : BLACK;
    }
    // TODO: PASS (置ける場所がない)、ゲーム終了の判定を追加すること!
}

onload = function() {
    // 黒と白の初期配置
    field[4][5] = BLACK;
    field[5][4] = BLACK;
    field[4][4] = WHITE;
    field[5][5] = WHITE;
    showBoard();
};

蛇足:
https://github.com/katoy/sample-enchant から
enchantjs で作ったオセロ http://homepage2.nifty.com/youichi_kato/src/github/enchant/reversi/index.html を参照できます。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/10/12 16:35

    なんと!質問を投稿してからこの僅かの間に、自分の書いたコードをベースにして、
    新たな機能まで追加してくださるとは!感謝もそうですが、驚きのほうが大きいです。

    加えて、ソースコードがとても読みやすいです。
    特に、field配列にint型のみが格納されている部分は是非とも参考にしたいところ、
    ここは自分でコードを書いていても、かなり気に掛かっていた部分でした。

    とはいえ、最初からこのソースコードを見るようなことはしません。
    もう一度、自分なりに修正を加えていく上で、その答え合わせのような位置付けで
    参考にしたいと思います。本当にありがとうございました。本当にお疲れ様でした。

    キャンセル

  • 2015/10/12 16:52

    改良したコードには "新たな機能まで追加" はしていません。
    追加するとよいと思うことを列挙しているだけです。
    the_saidaa さんが修正・機能追加したコードを是非再投稿していただけるると、さらにまたいろいろ指摘やコメントが集まると思います。
    (個人的には javascript でなく coffeescript で書くと、コードが短くなるとおもう)

    キャンセル

checkベストアンサー

+2

拝読させていただいたところ、真っ先に浮かんだのはCheckAble関数の修正です。

まずは、条件分岐について。
「参照先は壁か」「参照先は空か」「参照先には自分の石があるか」という条件は、このアルゴリズムにおいて頻出し、かつ重要なものです。
なので、これらを全て関数にして共通化し、かつ名前を付けて分かりやすくしてみましょう。
「参照先は壁か」→isWall
「参照先は空か」→isEmpty
「参照先に自分の石があるか」→isExistMine

javascriptでも同様かわかりませんが、私が良く書くJavaやC#では、真偽値(true/false)を返す関数(メソッド)にはisを先頭に付ける、という慣習があります(Rubyでは?を最後に付けます。言語によって違いがあるようです)。

変数およびメソッドの名前も、改良の余地がありそうです。
自分の石を表す変数としてstoneが利用されていますが、単に「石」と呼ぶ場合、それは自分の石と相手の石どちらとも取れます。ここでは、stone = 自分の石として用いているようなので、その意図が一見して分かるようにすると、可読性が上がります。
例)stone → myStone

石をひっくり返すかを判定する関数として、checkAble(可能か確認する)という名前をつけていますが、これだけだと「何について」可能か確認するのか分かりません。これも、意図を込めた名前にすると良いでしょう。
例)checkAble→checkReversible

最後に、checkAble関数の仕事を確認してみましょう。checkAbleは、名前の上では「ひっくり返せる石があるかどうかを確認すること」が仕事ですが、コードでは「実際に石をひっくり返す」作業も担当しています。
今回のように、一つのメソッドに複数の作業を混在させ、かつ同一のデータ(ここではfield[n][m])を各作業で利用すると、分岐条件や石をひっくり返すロジックに修正が生じた場合、互いに影響を与えてしまう恐れがあります。すると、修正範囲が広がったり、バグの原因が特定しにくくなったりします。
(結構面倒そうなので)ここでは省略させて頂きますが、実際にはcheckAbleを二つの関数に分けてみると、各処理毎に影響範囲を限定できます。挑戦してみてください。
例)
checkAble → isReversibleとreverseに分割

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/10/11 20:26 編集

    稚拙なプログラムを最後まで解読していただき、本当にありがとうございました。
    処理についてはもちろんそうですが、主に名付けに対してのご指摘が多く、
    その重要性について再確認させられたところです。

    指摘された点を修正し、もう一度だけ添削をお願いしたいと思います。
    重ねて、ありがとうございました。

    キャンセル

+2

もう解決済みですが個人的に思ったことを。
即時関数内に閉じ込めることでグローバル変数を減らせます。
次にDOM操作に関して、ループ内でDOMツリーに追加するのではなく、DocumentFragmentを使用し最後にまとめて追加することでパフォーマンスが上がるかと。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/10/12 16:19

    >即時関数内に閉じ込めることでグローバル変数を減らせます
    確かに、「とりあえず動けばいいや」的な考えで、何の考えもなしに
    グローバル変数を定義してしまったものは多いです・・・以後、気を付けます。

    >DocumentFragmentを使用し最後にまとめて追加することでパフォーマンスが上がる
    お恥ずかしながら、この回答を見るまでDocumentFragmentの存在を
    知りませんでした・・・逐一、8×8の64回に分けて要素を追加するよりも、
    1回にまとめて追加したほうが断然いいですね。

    Cf_cwdさんの回答だけでも、一気に知識の幅が広がったように思います。
    解決済みの質問にまでコメントを寄せていただき、本当にありがとうございました。

    キャンセル

  • 2015/10/12 23:14

    いえいえ。あとは思い付きですが、石は恐らく共通だと思うので一度作成した物をcloneNode()を使用してコピーするようにするともしかするとパフォーマンスが上がるかもしれません。(DOM操作の回数が減るので)
    あと今さらっとコードを見ていて、複数のstyleを変更している箇所があることに気が付きました。これはsetAttribute("style",value)の形にすることでスマートにできますね。

    ですからあらかじめsetAttribute()したdiv要素を作成しておき、ループ内でcloneNode()するようにするとよさそうですね

    キャンセル

  • 2015/10/13 00:07

    ちょっとちゃんと読んでみるとsetAttribute()だとfloatをclearする時、不都合があるかな?検証してないな……

    キャンセル

  • 2015/10/13 00:43

    三項演算子後のセミコロンがないことが気になります、またこういった書き換えができるかと。
    turn == 1 ? stone = black : stone = white

    stone = (turn == 1) ? black : white;

    先ほど色々と自己流に書き換えてみたのですが不具合のようなものが出ていて現状出せません。(かなり大雑把に弄ったことによって細かな抜けがあると予想)
    ですので変更した内容を箇条書きに。
    ・全ての変数を即時関数内に閉じ込めた(クロージャとして動作する)
    ・turnをturnStateと変更し、黒を1、白を-1として扱う
    (そうすることで黒と白を入れ替えるとき
    turnState = -turnState;
    と書くことで実現できる)
    ・backgroundImageというオブジェクトを定義
    ・画像をbackgroundImage.black、backgroundImage.whiteに代入
    ・isWall()、isMyStone()、backgroundImage.isNull()といった関数を定義することで、if文内を整理できます
    ・三項演算子を整理
    ・cloneNode()、setAttribute()による方法が動作することを確認

    他にもありますが連投かつ、長文ですのでここまで

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 88.04%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る