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

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

ただいまの
回答率

91.05%

  • JavaScript

    13309questions

    JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

  • jQuery

    5613questions

    jQueryは、JavaScriptライブラリのひとつです。 簡単な記述で、JavaScriptコードを実行できるように設計されています。 2006年1月に、ジョン・レシグが発表しました。 jQueryは独特の記述法を用いており、機能のほとんどは「$関数」や「jQueryオブジェクト」のメソッドとして定義されています。

  • オブジェクト指向

    239questions

    オブジェクト指向プログラミング(Object-oriented programming;OOP)は「オブジェクト」を使用するプログラミングの概念です。オブジェクト指向プログラムは、カプセル化(情報隠蔽)とポリモーフィズム(多態性)で構成されています。

メイン処理にチェック処理を記述しないデザインパターン

解決済

回答 3

投稿 編集

  • 評価
  • クリップ 1
  • VIEW 305
退会済みユーザー

退会済みユーザー

デザインパターンについて、まだ体系的に学んでいない初心者です
(シングルトン云々言われても、そういう手法があるんだな程度です)。
また、過去の質問もあまり見返していないため、重複していたら申し訳ありません……

現在の処理は、以下のような形となっています。

$(function(){
  // メイン処理
  main();

  function main() {
    // 処理Aを実行する
    processA();

    // プロセスAで実行した内容が正しいかチェックする
    isCheckedProcessA();

    // 処理Bを実行する
    processB();

    // プロセスBで実行した内容が正しいかチェックする
    isCheckedProcessB();

    // 処理Cを実行する
    processC();
  }
});


何だか、チェック処理が間に一々挟まって鬱陶しい気がしてます……
そこで、以下のような形にしたいと考えました。

▽理想

$(function(){
  // メイン処理
  main();

  function main() {
    try {
      processA(); // 処理Aを実行する
      processB(); // 処理Bを実行する
      processC(); // 処理Cを実行する
    } 
    catch(processAEroor) {      
    } 
    catch(processBEroor) {      
    } 
  }
});


これだとすっきりするんですけど、「じゃあどこでチェック処理やるの?」と考えたとき、
例えばprocessA処理中にエラーが発生した場合、processAの処理中で例外を飛ばすくらいしか
解決策が今のところ思い付きません……

processAのメソッド内のイメージ

function processA() {
  // 本来のprocessAの処理
  // ……
  // isCheckedProcessAの処理
}


「いやいや、それって単にisCheckedProcessAprocessAに入れただけやんけ!」
ってなってます(´・ω・)
「メイン処理とチェック処理を本来分けてたものを、一緒のメソッドに入れると分かりにくそう」
ってなってます(´・ω・)

そこで質問なのですが、処理を行うごとに入るチェック処理と
実際の処理を明確に分けたい場合、どのように記述すればいいですか?

ご教示のほど、よろしくお願いいたします。


▽以下、修正&加筆:

具体例に関して、修正依頼をいただきましたので、加筆いたします。
実際に、オセロのプログラムを組んでいてこの問題に直面しているため、それを例として挙げます。

$(function(){

  $('li').click(function() {
    // 選択された箇所のx座標、y座標を取得
    var y = getPutY();
    var x = getPutX();

    // 相手の石がひっくり返せる座標を取得
    getReversibleCoordinate();

    /**
     * ここでチェック処理を入れないと、相手の石がひっくり返せなくても
     * 自分の石が置けたり、手番が交代したりしてしまいます。 
     */
    if (isErrorPutStone()) { return; }

    // 選択箇所に自分の石を置く
    putStone(y, x);

    // 相手の石をひっくり返す
    reversibleEnemyStone();

    // 手番を交代する
    turnChange();
  }

  function getReversibleCoordinate() {
    // 相手の石がひっくり返せる座標を取得
  }

  function isErrorPutStone() {
    // 相手の石がひっくり返せる座標を取得できなかった場合を検証
    // 自分の石が置けなかった場合を検証
  }
});


……なんだか、元の質問とは別の部分を指摘されそうですが。。
ロジックの組み方に関して、ツッコミ所があれば、ご指摘いただけると助かります(´・ω・)

  • 気になる質問をクリップする

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

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

    クリップを取り消します

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

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

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

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

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

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

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

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

    質問の評価を下げる

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

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

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

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

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

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

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

    詳細な説明はこちら

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

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

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

質問への追記・修正、ベストアンサー選択の依頼

  • think49

    2018/01/03 22:04

    何の為に処理を分割するのでしょう。分割することが目的ではないはずで、最終的に到達したい動作を明らかにした方が良いと思います。また、チェック処理も具体性がないので全体像を想像しづらいように思います。

    キャンセル

  • 退会済みユーザー

    退会済みユーザー

    2018/01/03 22:53

    修正依頼ありがとうございます。「分割することが目的ではない」というのは、確かに仰る通りです。……なのですが、この字面通り受け取ったとき「(ケースバイケースで)特に変える必要ないのかな?」と、曲解してしまいました。また、汎用的な書き方が却って分かり辛かったようで申し訳ありません……具体例は後ほど追記いたします。

    キャンセル

  • think49

    2018/01/03 23:03

    "「(ケースバイケースで)特に変える必要ないのかな?」" 直観的には、変更前/変更後共に私は「使いたくないコード」と思ってしまいました。 processA() を実行後に isCheckedProcessA() は実行しなければならないのか、実行しなければならないなら別の名前に分離させる意味があるのか(因果関係があるなら、同じ名前空間に属するべきでは。分離するなら、独立させるメリットが必要。)。 try-catchはエラー処理で全て例外を投げなければならないのか、例外を投げられなかった場合は考慮しないのか(例外を投げないエラーを完全になくすのか)、など。

    キャンセル

  • 退会済みユーザー

    退会済みユーザー

    2018/01/03 23:45

    返信ありがとうございます。すべてのご指摘を理解できたわけではありませんが、「理想的なコードが組めていたら、そもそもこのような書き方にはならない」と解釈いたしました。具体例を追記いたしましたので、そこでまたご指摘いただけると助かります。

    キャンセル

回答 3

+3

 関数名

実際に、オセロのプログラムを組んでいてこの問題に直面しているため、それを例として挙げます。

processA() と isCheckedProcessA() がセットで実行されるように見えたのですが、

  • processA -> getReversibleCoordinate 
  • isCheckedProcessA -> isErrorPutStone

このような対応関係なら、違いますね。

どうも関数名が好ましくない気がします。
例えば、isErrorPutStone を reversible (ひっくりかえせるか真偽値を返す関数) に名称変更してみてはいかがでしょうか。

 参照透過性

基本的に、特別な理由がなければ参照透過性を持った関数が好ましいと感じます。
質問者さんのコードはざっくり書くとこんな感じです。

(function () {
  var x = 1, y = 2;

  A();
  B();
  C();

  function A () {
    console.log(x, y);
  }

  function B () {
    ++x;
  }

  function C () {
    ++y;
  }
}());

これはグローバル変数を使って、変数を共有しているのと実質的に同じです。
引数は全く使わず、参照透過性を完全になくしてしまっています。
せめて、次のように書きましょう。

(function () {
  function console (x, y) {
    console.log(x, y);
  }

  function incrementX (x) {
    return ++x;
  }

  function incrementY (y) {
    return ++y;
  }

  function main () {
    var x = 1, y = 2;

    console(x, y);
    x = incrementX(x);
    y = incrementY(y);
  }

  main();
}());
  • 入力値は出来るだけ引数で渡してください。
  • 出力は原則として、返り値に持たせて下さい。
  • 関数名は単体としてみても意味の通る名前にして下さい。
    関数Aに付属するcheckAなど、もってのほかです。
    チェックするのなら何をどのようにチェックするのか、具体的な処理内容を踏まえた名前にしましょう。

 改善案

無駄を排除していくと、次のコードを書けると思います。

jQuery(function(jQuery) {
  function getReversibleCoordinateList (x, y) {
    // 指定した座標及び相手の石がひっくり返せる座標を配列で返す
  }

  function setStone(color, coordinateList) {
    // 対象の全座標を指定色の石で埋める
  }

  function handleClick (event) {
    var li = event.target;  // クリックされた要素を取得

    // 選択された箇所のx座標、y座標を取得(data-x, data-y 属性に予め、座標を埋め込んでおく)
    var x = li.dataset.x;
    var y = li.dataset.y;

    // 相手の石がひっくり返せる座標を取得 (石を置いた座標も合わせて取得)
    var reversibleCoordinateList = getReversibleCoordinate(x, y);

    // 座標の数が2未満なら強制終了 (座標の数が1なら石を置いた座標しか得られていないので、ひっくり返る石は0)
    if (reversibleCoordinateList.length < 2) {
      return;
    }

    var data = event.data,
        color = data.color;

    // 対象座標を指定色の石で埋める(石を置き、相手の石をひっくり返す)
    putStone(color, reversibleCoordinateList);

    // 手番を交代する
    data.color = data.color === 'black' ? 'white' : 'black';
  }

  jQuery('li').on('click', {color: 'black'}, handleClick);
});

元のコードを尊重して「座標」としましたが、変数 reversibleCoordinateList は「li要素ノードの配列」にすると反転処理が実装しやすいと思います。

今後の発展形としては「classパターン」にすると、ページ上にオセロがいくつも作れて便利ですね。

Re: icicle さん

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2018/01/05 13:11 編集

    具体的なオセロの修正案含め、回答いただき本当にありがとうございます。
    なるほど……ひっくり返せる座標は全部まとめて一つに入れてしまえばいいんですね。
    言われてみれば確かに、となるのですが……一人で思い付くのは難しいですね……

    ご提示いただいた改善案で、もう一度組み直してみようと思います。
    コード修正後、改めてコードレビュー依頼の旨の質問を行うかもしれません。

    キャンセル

  • 2018/01/05 18:37

    こちらを参考に実際に組んでいる途中ですが、質問前と比較にならないほど
    コードがすっきりして驚きました。。既に50行近く削れた気がします。
    関数名の付け方だけでここまで変わるものなんですね……

    所感コメント失礼しました。

    キャンセル

checkベストアンサー

+1

これでいいのでは?

  $('li').click(function() {
    // 選択された箇所のx座標、y座標を取得
    var y = getPutY();
    var x = getPutX();

    if (!isStonePutable(y, x)) { return; }

    // 選択箇所に自分の石を置く
    putStone(y, x);

    // 相手の石をひっくり返す
    reversibleEnemyStone();

    // 手番を交代する
    turnChange();
  });
  function isStonePutable(y, x) {
    // x, yの値が正常であるか検証(getPutX/Y()で異常値が返らないのであれば不要)
    // x, yに置いたら相手の石をひっくり返せるか確認
  }

「x, yに置いたら相手の石をひっくり返せるか確認」はgetReversibleCoordinateを少しいじればいい感じにできるのではと思います。
(あり得る座標を総ざらいして確認していると思いますので、その総ざらいをやめて、座標一つに確認を絞ればよいのではと。)

さて、質問の主題ですが、

    // 処理Aを実行する
    processA();
    // プロセスAで実行した内容が正しいかチェックする
    isCheckedProcessA();

こういう場合、メイン処理とチェック処理は特に理由がないのであれば普通は分けないです。
後で(または他人が)コードを読むときに、行ったり来たりしなければいけないので、面倒です。
分ける理由は、複数のタイミングでチェックを行いたい場合等でしょうか。

特に理由がなければ、processAの処理の節目節目でチェックを行うと思います。
チェックが失敗した時点で後続の処理をキャンセルしたほうが、無駄な処理も減ります。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2018/01/05 12:58

    個人的に、最も汎用的に使える考え方を詳細に解説いただけたように
    感じましたので、今回のベストアンサーとしたいと思います。
    回答いただき、本当にありがとうございました。

    キャンセル

+1

 processA();
 isCheckedProcessA();
 processB();

processB() の前に必ず isCheckedProcessA() が必要なのであれば、チェックを processA() の末尾ではなく processB() の始めに置きましょう。

これをガードと言います。
つまり次のようになります。

function processB() {
 // isCheckedProcessAの処理(ガード)
 // 本来のprocessBの処理
 }

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2018/01/05 13:01

    回答ありがとうございました。
    ガード、という手法は初耳でした。言われてみれば、末尾に書くよりも
    先頭に書いたほうが分かりやすいですね……早速、今回のオセロにも取り入れるかもしれません。

    キャンセル

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

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

関連した質問

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

  • JavaScript

    13309questions

    JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

  • jQuery

    5613questions

    jQueryは、JavaScriptライブラリのひとつです。 簡単な記述で、JavaScriptコードを実行できるように設計されています。 2006年1月に、ジョン・レシグが発表しました。 jQueryは独特の記述法を用いており、機能のほとんどは「$関数」や「jQueryオブジェクト」のメソッドとして定義されています。

  • オブジェクト指向

    239questions

    オブジェクト指向プログラミング(Object-oriented programming;OOP)は「オブジェクト」を使用するプログラミングの概念です。オブジェクト指向プログラムは、カプセル化(情報隠蔽)とポリモーフィズム(多態性)で構成されています。