jQuery Promiseのthen()チェーンのお作法について

Question

非同期処理に関してthenを使うことで直列実行できることを最近知りました。
そのthenを用いて下記のように実装しましたが、検索上のサンプルコードでは繋げて書く基本的なコードしか
紹介されているのを見たことがなかったので、自分の実装に不安があります。
そこで、then()を含めメソッドチェーンによる直列処理についていくつか疑問があります。

1. then()内にthen()を書くのは一般的ではないのでしょうか？

その場合、途中で正常処理による中断を行いたい場合はどのようにするのでしょうか？

0. いくつかのthenチェーンを含む関数（loadSessionDataA()のような）を定義し、
その関数から派生させてthen()をチェーンさせてくべきではなのでしょうか？

0. then()内に定義する関数ですがこれは外出しして定義するべきなのでしょうか？
また、そのほうが可読性が良いですか？
個人的にはthen内に定義するほうが処理を上から順に追っていけますし、変数名を考えなくて済むので楽なんですが・・・

上記を踏まえて下記のコードをこうしたほうが良い、ああしたほうが良いなどあればご指摘いただければ幸いです。

※下記のコードは実際の処理をざっくり書いています

function onBtnClick() {
    mainFanction();
}

var mainFanction = function () {
    var mainPromise = $.ajax()                      //サーバに業務ロジックを依頼します
        .then(checkError, onAjaxFail)
        .then(function (responseData) {

            // Do Something

            if (responseData.warningCode) {         // 業務ロジックから受け取ったデータに警告コードがある場合
                isConfirmed = showYesNoDialog();    // ユーザに「～が〇〇です。このまま処理を実行しますか？」的なダイアログを表示

                if (!isConfirmed) {
                    deferred.resolve();
                    return deferred.promise();      // この場合なにもせずに処理を中断する(alwaysは通す)
                }
            }

            if (local_data.flg) {                   // ローカルのなんかのフラグがtrueの場合 
                isConfirmed = showYesNoDialog();    // ユーザに「～が〇〇ですがよろしいですか？」的なダイアログを表示

                if (!isConfirmed) {
                    return syncWithSessionDataA();  // ローカルデータAとセッションデータAを同期して処理を中断する
                }
            }

            // Do something

            var subPromise = syncWithSessionDataA() // ローカルデータAとセッションデータAを同期する
                .then(syncWithSessionDataB)         // ローカルデータBとセッションデータBを同期する
                .then(function () {
                    $('form').submit();             // 画面遷移を行う

                    deferred.resolve();
                    return deferred.promise();
                });

            return subPromise;
        })
        .fail(function (errCode) {
            showErrorDialog(errCode);
        })
        .always(function () {
            // Do something
        });

    return mainPromise;
};

// これより下のコードは共通化したい意図があります。

// SessionDataBも同じロジックです
function saveSessionDataA() {
    return $.ajax().then(checkError, onAjaxFail);   // ローカルデータAをサーバに送り、セッションへの格納を依頼する
}

function loadSessionDataA() {
    var prom = $.ajax()                             // サーバにセッションデータAを要求する
        .then(checkError, onAjaxFail)
        .then(function (responseData) {
            local_dataA = responseData              // ローカルデータAにセッションデータAを上書く
        });

    return prom;
}

function syncWithSessionDataA() {
    return saveSessionDataA().then(loadSessionDataA); // ローカルデータAをセッションに格納した後にローカルデータを最新化する
}

// 業務ロジックのエラーを判定するロジック
function checkError(responseData) {
    if (responseData.isError) {　　　　　　　　　// 業務ロジック内で例えばExceptionなどが発生した場合にisErrorはtrueになります
        deferred.reject(responseData.errCode);
    } else {
        deferred.resolve(responseData);
    }
    return deferred.promise();
}

// Ajaxエラーをエラーコードに変換する関数
function onAjaxFail(xhr, status, error) {
    var errCode;
    switch (status) {
        case 'timeout':
            errCode = 'XXXXXX';
            break;
        case 'error':
            errCode = 'YYYYYY';
            break;
    }

    deferred.reject(errCode);
    return deferred.promise();
}

補足情報（FW/ツールのバージョンなど）

IE11
ECMAScript 5
jquery-2.2.4

追記

冒頭の書き出しに関して誤解が生じてたので修正しました。

Comments

[spookybird]

指摘してくれた上司に理由を確認すべきだと思います。なぜイケてないのか、thenの意図した使い方とはなんなのか。指摘するからには理由があると思うので。それを自分で考えみろってことまで含めての指摘なら、もう少しご自分でしっかり学んで考えてみた方がいいと思います。理由もなくなんとなく指摘されているだけなら、今のコードが良いとか悪いとか以前に上司がアホなのでほっといていいと思います。

[sakata_inu]

コメントありがとうございます。実を言えば上司もここ最近にthenによって直列実行ができることを知りました。 なので私も含めthenのサンプルしか見たことがなく実際の業務でどのように使われているかは分かりません。 その上で、上司は、thenを繋げて書くことでその順番に処理が実行されることを保証するための関数という解釈の元、 その繋がりが途切れるような、thenの中にthenを書くだとか、thenの処理をまとめて書いた関数からthenを繋げてかくだとか は、処理の順番が分かりづらくなりthenを使う意味がないのではないか、というのを理由に指摘しています。 私も同じ解釈ですが、指摘理由に関しては疑問を思うところがあったのでこのように質問させていただきました。

Answer 1

then()内にthen()を書くのは一般的ではないのでしょうか？
その場合、途中で正常処理による中断を行いたい場合はどのようにするのでしょうか？

then()内にthen()を書くというよりは、then()内で別の非同期処理を実行するという表現になるかと思いますが、別に何も問題はありません。
一般的かどうかと問われると知りません。
そう書かざるを得ないなら書きますし、別の手段で書けそうなら別の手段で書いてもいいです。

また、「途中で正常処理による中断」というのがよくわかりません。
もう少し具体的にどうしたいのか整理してみたらいいと思います。
自分が何をどうしたいのかはっきりさせていけば、他の書き方も思いつくかもしれません。
最低限「何を」「いつ」「どこで」「どのように」するのか、「なぜ」そうしたいのかを明確にして、日本語で書いたものを可能な限り関数名等に置き換えていくといいと思います。

いくつかのthenチェーンを含む関数（loadSessionDataA()のような）を定義し、
その関数から派生させてthen()をチェーンさせてくべきではなのでしょうか？

「その関数から派生させて」の意味がよくわかりません。

then()内に定義する関数ですがこれは外出しして定義するべきなのでしょうか？
また、そのほうが可読性が良いですか？
個人的にはthen内に定義するほうが処理を上から順に追っていけますし、変数名を考えなくて済むので楽なんですが・・・

「可読性」はわりと好みが出ます。
「個人的には」ではなく、チームや組織のルールに従うのがベストです。
ルールがないならチーム内で作りましょう。
よくあるルールとしては、1つの関数内の行数XXX行以下、とかですかね。
長くなりすぎるなら切り出すし、短いならそのまま書いちゃうといいです。
切り出し方は個人で好きにすればいいと思います。
エラー処理だけ別で切り出すとかもよくある方法です。

---

極論を言えば、意図した通り大きなバグもなく動いているならそのソースコードは間違っていません。
理想がどうとか、読みやすさがどうとかは個々人の価値観によって違ってきますので、絶対的な正解を提示することは不可能です。
例えば私が仕事上で行うソースコードレビューにしても、修正は「提案」であって「強制」ではありません。
レビューコメントから議論が起きて検証合戦がはじまったりもします。

非同期処理が完璧に意図通りに動いているかの検証はなかなか難しいところもありますが、例えば一部の処理を極端に遅くしたときに意図した順で動いているかとか、全部の処理を極端に遅く（通信なら通信自体を遅くする）したときに問題ないかとか、非同期処理内でエラーが発生したときに意図した挙動になるかをひとつずつ順に見ていくとか、そうやって事細かに全パターンのテストをしていけば、同時に処理の流れもわかってきます。
そのうえで、ここはこう書き換えた方がテストしやすいとか、後日自分で読んで読みにくかったところを修正するだとか、そういうこともできるかと思います。

Answer 2

結局その上司のレビューを通ることになるので赤の他人に聞いてどうこうなるものではないと思います。
その上司（というかレビュワー全てに言える）に意図や対応の方向性の確認をするしかないと思います。

例えばここで何かしらの案が提示されたとして、上司にレビューを頼んだとしましょう。
「どのような意図で修正したか」きちんと語れなければいけません。
もしかしたら「なぜこちら（レビュワー）に修正方針を確認せずに勝手に修正したんだ」と更に指摘を喰らうかもしれません。
だからこそ他人には非常に手を出しにくい内容です。無責任に「こうですよ」と答えられるものではありません。

一般的な常識と個々の常識と現場の常識は必ずしも一致するとは限りません。
特に現場は「ローカルルール」が優先されることもあるので。

Comments

[sakata_inu]

回答有り難うございます。本文の冒頭によって誤解を生じさせてしまい申し訳ありません。あくまで本投稿はthenの正しい使い方を質問しているものであり、
開発の方針や方向性を疑問視しているものではないことをご了承いただければと思います。もとより非同期処理の直列実行をどのように実現するかの方針は
決まっておらず、当初、コールバックを使うか、ajax.asncをfalseにするかを検討していたところ、thenの存在を上司含め私も知ったところです。
thenを検索したサンプルでは基本的な使い方しか紹介されておらず、私のコードのような使い方に関して、私も不安なところがありますので、質問の内容や、コードに関してご回答いただければ幸いです。

[m.ts10806]

回答に追記しようと思ったのですが、考えていたことと同じことが回答としてついたこともあるので、やめておきます。
実際に、要件を満たす形で不具合なく動作しているコードであればそれは問題ないと考えられます。
ご自身がどのような意図を以てそういう実装をしたのかを上司に説明し、納得してもらえるように持っていくだけかなと。
特に実装方針が決まっていない場合は、自分でルールを作れる可能性があるので、むしろチャンスですね。
動作検証をきちんと行い、要件を満たすことを確認したらきちんと説明して了承を得て進めて良いとは思います。
ルールが不安定で、しっかりと組みあがっていない段階でのリファクタリングはかなりのリスクを伴います。それであれば、実現可能な方法で実装を行い、組みあがり、ルールも決めていった後に、可読性や再利用性なども含めて検証して改修していけば良いと思います。
初めから完璧なコードが書けるわけでもないですし、実際に動いている以上はあれこれ指摘しても余計混乱するだけだとも思います（現状のコードにあれこれ指摘事項があるという意味ではないです）