Androidのダイアログの表示時のお作法

Question

###前提・実現したいこと
相談出来る方が居ないので色々とお作法を教えていだければと思います。
ストレートに言えば、サンプルに示すコードのコードレビューをして頂きたいです。

###疑問点
Androidではダイアログを表示する際にAlertDialogを直接使うのではなく、DialogFragmentを介して表示するのが一般的だと解釈しました。

呼び出し元のActivity（やFragment）側でのイベントの取得が面倒になるような気がしつつ、私なりに解釈した確認ダイアログの実装が下のコードです。

自分で駄目だと思っている点

• ダイアログを出すトリガーとなる箇所に、閉じたときのリスナーが直接書けないので可読性が悪い。

• 複数のダイアログを出した場合は、RequestCodeを貰うなど改造して、Positive、Negativeリスナーで処理の振り分けを書かなくてはいけない。
• 他の種類のダイアログ（アラートダイアログとか）作り、同じAcitivityで呼び出すようになると、もう収集がつかなくなりそう。
• その他、なんとなく直感的にコードが書けない。

###サンプルで作ったソースコード

public class ManyDialogActivity extends AppCompatActivity implements ConfirmDialogFragment.ConfirmDialogListener {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_many_dialog);

        Button button1 = (Button) findViewById(R.id.button1);
        button1.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                ConfirmDialogFragment dialog = ConfirmDialogFragment.newInstance(null, "Aを登録しますが良いですか？");
                dialog.show(getSupportFragmentManager(), "TAG");
            }
        });
        Button button2 = (Button) findViewById(R.id.button2);
        button2.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                ConfirmDialogFragment dialog = ConfirmDialogFragment.newInstance(null, "Bを登録しますが良いですか？");
                dialog.show(getSupportFragmentManager(), "TAG");
            }
        });
        Button button3 = (Button) findViewById(R.id.button3);
        button3.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                ConfirmDialogFragment dialog = ConfirmDialogFragment.newInstance(null, "Bを登録しますが良いですか？");
                dialog.show(getSupportFragmentManager(), "TAG");
            }
        });
    }

    @Override
    public void onClickPositive() {
        // 引数にRequestCodeのようなものを使って処理を振り分ける？
    }

    @Override
    public void onClickNegative() {
        // 引数にRequestCodeのようなものを使って処理を振り分ける？
    }
}

public class ConfirmDialogFragment extends DialogFragment {
    public static final String TITLE_KEY = ConfirmDialogFragment.class.getName() + ".TITLE_KEY";
    public static final String MESSAGE_KEY = ConfirmDialogFragment.class.getName() + ".MESSAGE_KEY";

    public static ConfirmDialogFragment newInstance(@Nullable String title, String message) {
        ConfirmDialogFragment fragment = new ConfirmDialogFragment();
        Bundle bundle = new Bundle();
        bundle.putString(TITLE_KEY, title);
        bundle.putString(MESSAGE_KEY, message);
        fragment.setArguments(bundle);
        return fragment;
    }

    @NonNull
    @Override
    public Dialog onCreateDialog(Bundle savedInstanceState) {
        Bundle arguments = getArguments();
        String title = arguments.getString(TITLE_KEY);
        String message = arguments.getString(MESSAGE_KEY);

        final Activity activity = getActivity();
        AlertDialog.Builder builder = new AlertDialog.Builder(activity)
                .setTitle(title)
                .setMessage(message)
                .setPositiveButton("OK", new DialogInterface.OnClickListener() {
                            @Override
                            public void onClick(DialogInterface dialog, int which) {
                                if (activity instanceof ConfirmDialogListener) {
                                    ((ConfirmDialogListener) activity).onClickPositive();
                                }
                            }
                        }
                )
                .setNegativeButton("キャンセル", new DialogInterface.OnClickListener() {
                            @Override
                            public void onClick(DialogInterface dialog, int which) {
                                if (activity instanceof ConfirmDialogListener) {
                                    ((ConfirmDialogListener) activity).onClickNegative();
                                }
                            }
                        }
                );
        return builder.create();
    }

    interface ConfirmDialogListener {
        void onClickPositive();

        void onClickNegative();
    }
}

###補足情報(言語/FW/ツール等のバージョンなど)

• Android4.4 から 7 までが一応の対象バージョンです。
• サンプルはJavaで掲示しましたが、Kotlinで書いてます。

Answer 1

Androidは魔境です。
DialogFragmentってあくまでダイアログ風なFragmentなんですよね。。
まぁ使いづらいのでAlertDialogと同じ事できるクラスは用意しておくとある程度楽ですが

コールバック周りはputSerializableとかで上で言うとnewInstance時にリスナー入れたりも一応できますが、回転とか考えると現状のやり方が私はいいかと。
あと個人的にはonAttachでリスナーセットされてるか処理します。

あまりにもリスナー多い場合はidなんか用意したりBundleとうまく向き合います。
なんかActivityResultやBroadCastに慣れるとimplementsが増えるくらい気にしなくなる。。

Comments

[type23]

ありがとうございます。
サンプルばかり読んでいて、実際のアプリのコードを読んだことがないので、どれだけ皆さんが苦しんでいるかが分からなかったです。

> Androidは魔境です。
割りとこの一言で救われた気がします。
たかがダイアログを表示して、処理を振り分けるのにここまで苦労するとは思いませんでした。
正直30分もあれば実装できると思ってたのですが、1日がかりです。
みなさんこんなもんなんですか？

> コールバック周りはputSerializableとかで上で言うとnewInstance時にリスナー入れたりも一応できますが
この手法を最初に考えついたのですが、私の設計が悪いのか、関連する全てのクラスをSerializableにしないといけなくなり、Serializableである必要のないクラスまでSerializableにしなくてはならなくなってしまいました。

> あと個人的にはonAttachでリスナーセットされてるか処理します。
後学のためにこちらの理由を教えて頂けますか？

> あまりにもリスナー多い場合はidなんか用意したりBundleとうまく向き合います。
Bundleについての理解が浅いようです。
先の方にもBundleに関してアドバイスを頂きましたが、理解が出来なかったです。
私がもう少し知見を深めなくては話が進まないですね。。。

[uniko]

うーん、とりあえずこちらの端末で動いたけどあっちで動かないとか普通にあるのでまず個人的には安定して動くような方法を重視していますね。ユーザーがどんな操作するか分からないので。。あとSupportLibraryとか更新するだけで落ちたりする場合もあるし。。。

onAttachでリスナーセットは
結局リスナーつけると言う事は返すと言う事なのでコールバックできないと意味ないので。最初についてるか判断してなければなければな処理をしますね。まーあまりないですが例えば不正な呼び出しですよとか。

試してはいませんがこちらの方がソース上げてるみたいですねputSerializable
https://gist.github.com/mstssk/7123411

[type23]

遅くなりました。ありがとうございます。

> まーあまりないですが例えば不正な呼び出しですよとか。
今はリスナーをセットしなければ何もしない。という動きにしたかったので、現状のままでも大丈夫そうですね。

参考にあげて頂いたコードも見ました。
最初に思いついた手法でしたが、理解が浅いせいか期待の動きをしなかったので、今の手法を取り入れました。

コードをスッキリさせるためにKotlinで書き始めましたが、動くコードを書くうちに混沌としてきてしましました。。
Androidで製品を作る。というのは想像以上にコストが高いですね。

[uniko]

onAttachについてはライフサイクル的にもActivityに取り付けられた時なのでって意味でやってるとこもありますね。
特別なことすると大変な時もありますが慣れてくればクセもわかってくると思いますよ。

[type23]

ありがとうございます。
なるほど、恐らく誤解してました。
上手く説明が出来ないですが、確かにonAttachでチェックする方が良さそうな気がしてきました。

Answer 2

■改善点
・onCreateDialogのreturnはbuilderをcreateしたものでは？

■疑問に対する回答

ダイアログを出すトリガーとなる箇所に、閉じたときのリスナーが直接書けないので可読性が悪い。

これは、リスナーをダイアログフラグメント側のフィールドに持つとダイアログフラグメントがシステムから壊された時に復帰できないので現状の実装がベターなのかなと思いますを

複数のダイアログを出した場合は、RequestCodeを貰うなど改造して、Positive、Negativeリスナーで処理の振り分けを書かなくてはいけない。

他の種類のダイアログ（アラートダイアログとか）作り、同じAcitivityで呼び出すようになると、もう収集がつかなくなりそう。
その他、なんとなく直感的にコードが書けない。

これに関してはConfirmDialogListenerの各メソッドの引数にBundleを指定することで備えるといいですね。

Comments

[type23]

ありがとうございます。
ザックリ一言で言えば、今のところこのコードでも悪くない。という認識をもちました。

> ■改善点
> ・onCreateDialogのreturnはbuilderをcreateしたものでは？
失礼しました。ご指摘の通りです。動かしてから貼り付けるべきでした。

> これに関してはConfirmDialogListenerの各メソッドの引数にBundleを指定することで備えるといいですね。
パっと具体的なコードが思いつかなかったのですが、どのような感じでしょうか？

ご指摘のイメージから実験コードを書いた方が話が進みやすいかと思いましたが、ちょっと手間取りそうだったので、急ぎ聞かせていただきました。

[yona]

Activityで画面遷移する時にBundleにいろいろなデータを入れてやりとりしますよね？
あれと同じことをコールバックインターフェースで行うと、ダイアログクラスが増えてもコールバックインターフェースは１つで済みます。
Bundleに入れる値もしっかりと設計すればある程度保守性は保てるかなと思います。

[type23]

遅くなりました。ありがとうございます。
週末バタバタしてしまい、全く実験が出来なかったです。

> Bundleに入れる値もしっかりと設計すればある程度保守性は保てるかなと思います
なるほど。やはりこの辺りはしっかり腰を据えてやらないと難しそうですね。
結局ダイアログを閉じたときにViewをゴニョゴニョしたいことも多く、activity間の結合を疎にしたいと思ったときに、行き当たりばったりだと期待した動きにならず、無駄に労力を費やしてました。