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

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

ただいまの
回答率

89.10%

共通化してsimpleになったと思えますかね?

受付中

回答 8

投稿

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

退会済みユーザー

いつもお世話になります。
雑談というか相談です。
3つのFormで、同じような処理をする、13行くらいのmethodがあります。
このmethodには11個の引数を渡していて、それを渡せば、共通のmethodにできます。

public void commonMethod(引数11個){
//それぞれの引数に対する処理。

}

ということです。

これって、共通化してsimpleになったと思えますかね?
引数が11個という時点で、simpleとはいいがたい気がして、別々に実装したほうがいいかなと悩んでます。

引数を減らすためにclassを作るのも大げさだし。
こういうときって、みなさまなら、どうします。
雑談モードでお聞かせください。

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

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

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

    クリップを取り消します

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

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

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

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

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

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

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

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

    質問の評価を下げる

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

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

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

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

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

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

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

    詳細な説明はこちら

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

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

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

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

  • ozwk

    2015/11/30 14:55

    コメント欄に出ているクイズとは「次の1~nの(なにか)を~の順番に並べなさい」的な問題ですか?

    キャンセル

  • 退会済みユーザー

    退会済みユーザー

    2015/11/30 14:58

    クイズはサンプルで、コード例は単なる例です。

    この問題は、そういう問題ですね。

    キャンセル

  • ozwk

    2015/11/30 15:05

    他の形式のクイズと共通化しようとしていますか?例えばただ単に名称を答えさせるとか、n択問題とか。

    キャンセル

  • 退会済みユーザー

    退会済みユーザー

    2015/11/30 15:39

    いまはstringの比較ですむもの3例を共通にするかなー、と悩んでいます。

    ただ、今日、みなさまと楽しく討論していて、どうも今回はしないほうがよさそう、というところに落ち着きそうです。

    キャンセル

回答 8

+4

「重複するコードを括り出してはみたが、それを呼び出すコードが同じくらい複雑」現象、と一般化できますか。

一般論でお答えしますと、その現象にぶつかったときは「括り出すべきかどうか」を議論する意味はあまりなくて、もっと全体の処理フローがまずくないか、そちらを見ていく方が正しいアプローチです。

今回の話ですと、13行の処理が11個のパラメータを必要としている時点でデータの凝集度がいまいちで、オブジェクト同士が疎結合になっておらず今後もいろいろな苦しみに遭遇しそうであるなあと感じます。

とは言えあまりプロダクト設計全体の議論もここではできませんからその13行の処理についてもう一回フォーカスしてみますと、ラムダ式を渡す形にすることで引数をシンプルにできたりしませんか?

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 19:50

    他の回答に付いたコメントも読ませていただきました。
    「あまりフローは多くなくて、見直すところがあまりない感じ」なんてとんでもない、これはかなり見直すべきコードですとも。

    「リーダブルコード」はもう読まれました? http://www.amazon.co.jp/dp/4873115655 この本です。今回の件に直接役に立つ内容も含まれていますし、どちらにしろ通して読んでおいて損はありません。

    キャンセル

  • 2015/12/01 09:13

    えーほんとですか。

    精進します。

    キャンセル

  • 2015/12/01 12:41

    はい、改善したほうがいい点を端的に申しますと、このコードは
    ・入力のあったコントロールの識別
    ・フォームからの値の読み出し
    ・正解判定
    という別々のロジックを一度に記述しています。
    知識として言うと、
    ・Windows Formsというフレームワークの仕組みの知識
    ・フォームをどう構成したかという知識
    ・正解判定はどういうロジックで行われるかという知識
    というまったく別々の分野の知識が動員されています。これが「凝集度の低い」状態です。

    例えばですよ、「このアプリをWebアプリに移植したい。ASP.NETなら同じC#だから簡単でしょ?」と行くかというと、無理なんですよね。ほぼ一から書き直しです。
    「少しルールの違うクイズを追加したいんだけど」でもごっそり書き直しです。

    じゃあどうしたら移植や拡張に強い、凝集度の高いコードにできるか。ご紹介した本はその一助になるはずです。

    キャンセル

+3

フォームが異なっても、渡す引数が共通であるなら、シンプルになったんじゃないですか。

フォーム1では1番目から9番目の引数が使われ、フォーム2では1番目から5番目と8番目から11番目の引数が使われ、フォーム3では3番目から9番目の引数が使われる、というように「どのフォームに使うのかを意識して引数を決めなければならない」メソッドであるなら、混乱の種を撒いただけだと思います。

質問に書かれている『同じような処理』は、何が同じで何が異なっているのか次第じゃないですか。
それを、きれいに整理できないのであれば、1つのメソッドにまとめるのは誤りです。
きれいに整理できないという事は、動作の把握が困難なきたないコードを書かざるを得ないという事ですから。


投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 13:07

    なるほど。参考になりました。

    引数が多いので、いっそFormそれじたいをわたしてしまうか、などと考えていました。

    しかし、結局、おっしゃるように、わたしたもののどれを使うかは、別々に条件を立てて考えなければならないところがあり、混乱するな、とも思い、躊躇してもいます。

    キャンセル

  • 2015/11/30 13:22

    内部的に使用するものがケースによって違うのなら、まずそれぞれを分断してみてはどうでしょうか?
    ケースバイケースのようなコードが多数入ってる時点で共通化できてるとは言い難いと思います。

    キャンセル

  • 2015/11/30 14:33

    そうですね。


    おおまかなコードを載せました。

    ただ、問題によって回答数が変わったり、いろいろ違うこともあるので、どうしたものかなと。

    キャンセル

+2

こんにちは。

程度はあると思いますが、シンプルにはなったと思いますよ。
アプローチの仕方はケースバイケースですが、Formに実装された処理であればインスタンスメソッドでしょうか。
であれば抽象化した上で同じじゃない部分だけオーバーライドさせる方法もありますし。

静的メソッドであれば引数の渡し方は多少改善は出来るかもしれません。
構造体だったりクラスを引数として渡す。処理の内容によってはフォームインスタンスを渡す場合もあると思いますし、特定のインターフェイスを実装した上で引数として扱う場合もあると思います。

全ては共通化したい処理次第です。
でも、少なくとも複雑化はしてないと思いますよ。(同じ「ような」処理ってのが気になりますが…)

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 14:26

    おおまかなコードを載せました。



    ざっくりいうと、クイズのプログラムで、問題ごとに回答の数が微妙にかわったりすることがあります。



    コアを作って、問題のformはそのコアを継承しています。



    で正解の判定のところで、入力した回答のtextboxと正解かどうかを表示するcheckbox, それに正解の文字列(11個)を引数として渡せば、正解の判定は共通処理としてできるのではないか、と考えたわけです。

    キャンセル

  • 2015/12/01 12:07

    今ここだけ見たのですが・・・(全体的なコメントはこれから見ます。)
    委譲じゃなくて継承にしてる理由ってありますか?

    キャンセル

  • 2015/12/07 16:06

    こんにちは。
    わたしがあまりプログラムに詳しくなく、移譲を知らないためです。
    継承も、今回初めて使いました。

    キャンセル

+2

「リファクタリング」によれば

既知のオブジェクトに問い合わせることができる場合:「メソッドによる引数の置き換え」
複数の引数が1つのオブジェクトにまとまっていれば:「オブジェクトそのものの受け渡し」
引数をオブジェクトにまとめる:「引数オブジェクトの導入」

コメント欄や追加修正欄でのやりとりを踏まえてですが、
とりあえず画面ありきでプログラミングするのをやめましょう。
(私も昔はそうしていましたし、そう言われてもいまいちよくわかっていませんでしたが。)
画面に何でも実装するのは、「分けて実装するのが基本だが、面倒なので」という上級者向け行為です。

クイズの例で言えば、作りたいのはクイズであって画面ではありません。
クイズの出題/回答をする部品(クラス)を作ってから、それを操作する画面を作りましょう。

番号を順番に並べるクイズの例であれば、順番があっているかどうか判定できればいいのですから、
引数11個も必要なく、そもそもstring1つ(か何か)で済む話です。

これを画面ありきで作ると、
・解答欄がある
・何個あるかは解らない
・じゃあ複数ある場合で共通化しよう
・引数が11個になった
という謎の構造になります。

まずすべきことは問題の出題形式のグループ化です。
(順序を答える、単一の名称を答える、n択問題、可能なかぎり列挙する、などなど)
こういう状態を指して、「もっと全体の処理フローがまずくないか検討した方がいい」
と指摘されていると思います。

例のように、複数のテキストボックスに回答を入力し、複数のチェックボックスで正誤を表示するなら、
画面がテキストボックスの入力状況から回答文字列を生成し、クイズクラスに採点をお願いし、
結果を受け取ってチェックボックスを操作するのが「シンプル」だと思います。

また、現状ですと、例えば3択問題100個つくったら
subformX.csX=1~100作る気でいるようですが、
クイズクラスを作れば
・3択問題クラス
・3択問題回答用画面クラス
・3択問題を何らかの方法で100個生成するクラス
で済みますね。

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 17:00

    おお。なるほど。
    すこし目が覚めました。

    おっしゃるとおり、画面ありきで設計するのは微妙だなとは思います。
    ただ、言い訳するわけではないといいつつ言い訳ですが、諸事情により画面は既定なのです。
    所与で、そこを変更することは、今回はむずかしいです。

    わたしは設計に深くは関与しておらず、その画面制御のコアの部分は、画面と連動した外部周辺機器を操作しているので、手を加えるとしても、ミクロすぎて手出ししにくいとか。
    仕様がぜんぶ決まっていないとか、納期は決まっているとか、共通化しなくてもとりあえずは動いているからとか、そういう事情です。

    設計がよくないなあ、とは思うので、できるところはsimpleに、と思うのです。

    現時点では、問題が出そろっていないので、
    「まずすべきことは問題の出題形式のグループ化です。 」
    を行えないのです。

    さらに、採点クラスを作っても、そこの処理は判定だけではすまない、ということもあります。
    採点したら結果を表示するとか、他のチームと順番をつけるとか、まあ、それもフローのうちですけれど。

    もし集まっtailとかで雑談できれば、もうすこしお話しできますね。

    設計のこと、共通にするか、しないか。画面とロジックの分離、クラスなど、みなさまのご意見をお伺いできて、とても参考になり、楽しい時間を過ごせました。
    今回は変更なしになりそうですが、また次の機会には活かしていきたいと思います。
    ありがとうございました。

    キャンセル

  • 2015/11/30 17:16 編集

    > 現時点では、問題が出そろっていないので、
    「まずすべきことは問題の出題形式のグループ化です。 」
    を行えないのです。

    新しいパターンができてもなるべく既存の部分を変更しないために行います。


    > 画面制御のコアの部分は、画面と連動した外部周辺機器を操作しているので

    > 採点クラスを作っても、そこの処理は判定だけではすまない、ということもあります。
    採点したら結果を表示するとか、他のチームと順番をつけるとか

    ますます画面とロジック分離した方が見通しよくなるんじゃないかと思います。

    ともあれ開発頑張ってください。

    キャンセル

  • 2015/11/30 18:26

    ありがとうございます。

    DBは使う予定だったのですが、応答速度が3秒とかで、なしになり、csvでもつ、みたいになる話をしてます。

    いろいろ流動的な状況です。



    わたしも分離したほうがいいとは思うし、classつくるほうがとは思います。

    キャンセル

+1

設計を考えずに判断すると、メソッドに切り出し出来るのならした方が良いと思います。
同じコードはないほうがいいからです。

設計的な視点から見ると、そのコードを共通化するメリット・デメリットを挙げてみたらどうでしょうか?
・これから共通コードが再利用される可能性はどの程度あるのか
・以降、共通コードが変更、または増えそうな見通しはあるか
・『単純に同じ処理してるから』で括ってないか? 『たまたま同じ処理』ということはないか?
・今は共通だがもしかしたら共通でなくなる可能性はないか
・13行を共通化するとトータルでコードがどれだけ減るのか(逆に引数11個セットするのにコストかかってないか)

引数が多い問題はクラスで渡すほうがいいと思います。
パラメータークラスを作る手間を惜しんでいる方をたまに見かけるのですが、そのクラスを作るメリット・デメリットを挙げてから判断してみては如何でしょうか?
要はそのクラスを作る、保守するコストに見合ったものが得られるかどうかです。

設計的に見たら、引数が多い問題は見なおしてみたほうが良いと思います。
例えば共通メソッドにせず、クラス化して切り出したらどうでしょうか。
この場合、メソッドの引数には本当にその時必要な引数のみを渡すことが出来ることが多いです。

例えば引数にTextBoxを渡してメソッド内でTextプロパティを変更しているような場合、TextBoxそのものは予めクラスに渡しておけますよね。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 16:57 編集

    正解判定はクラス内の一箇所でしかしていないですよ。Formでは一切行っていません。

    Loadは初期設定(テキストボックスとそれに対応した答えの設定)のみです。
    この時に答えを設定しているのですがForm_Load時以外でも何度も問題が差し替わるということでしょうか?
    私はForm一つにつき問題が確定しているという認識で作っていました。
    (なのでLoad時に一回問題と回答をセットすれば良いのかなと)

    もしTextBox数がダイナミックに可変するようなら、新たな問題を設定する時に再度初期化すればいいだけだと思います。

    キャンセル

  • 2015/11/30 17:04

    あ。そうではなくて、いま複数のFormがあって、変更のたびに初期設定を行うのは面倒だ、ということです。
    もし集まっtailとかで雑談できれば、もうすこしお話しできますね。

    キャンセル

  • 2015/11/30 17:08

    ・Form毎に設問設定するのが面倒
    ということですね。
    この辺りはDBなどに設定しておいて勝手に突っ込むとかの実装をすれば初期化コードも共通化出来ると思います。

    残念ですが私は集まっtailは不参加なのです。

    キャンセル

+1

どんな状況下でも
   (引数11個)
は、却下ですね。

シンプルになったかどうかの評価は、ハッシュやテーブルを使うなどして 引数が 4個程度になってからです。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/01 09:12

    おお。やっぱりそうですね。

    わたしも引数を数えて、これはないわーと、みなさまのご意見をうかがってみたかったのでした。

    ほんと、引数11個はないわー。

    ですね。

    ありがとうございます。



    結局、現状のままいじらずいこう、というほうに傾きました。



    あと、補足的になりますが、今回の場合、クイズの回答ロジックの部分を共通化したいのではなく、回答にまつわるいろいろ(正解の時にピンポン、不正解のときにブブー)みたいな、がめん周りを含めて共通化したい、そこまでしないと共通化の意味(コードが減る)がない、ということがあります。

    ロジックとインターフェイスをわける、というのはひとつの考え方で、それにはわたしも共感しますが、今回の場合には残念ながらロジックとインターフェイスが込みというか、ロジックだけならほんの数行、インターフェイスの共通化のほうが重要、という感じです。

    このあたりをうまく説明できなかったのですが。

    インターフェイスの抽象化は、継承を使ってそれなりに行ったつもりですが、もう1段階上の抽象化を今後は検討します。

    キャンセル

+1

実務では他人のソフトをメンテナンスすることが(なぜか)多い自分ですが,引数11個,というのは失笑モノ,というレベルですね(残念ながら実装されているケースはあります…).

書き込まれた内容と少しずれますし,今こういうふうに設変かけられるわけじゃないのは承知していますから,ここから先は遊び半分でお読みください.

(ガッツリと問題クラスを実装するほうがメリットが大きい,というのは別として)簡易実装でこんなの考えてみました.
以下の様な構造体
・問題文
・正答(配列1番目)
・誤答(配列2番目以降,たとえば最大5件までで,誤答数は不定,最後は空文字列)

これを構造体配列として問題集を作ります(フォームを表示する前とか,問題集を取り込むタイミングで生成します).フォームへは構造体の1要素を渡し,設問を出す方のフォームで問題文を表示,解答一覧をランダムで散らす,という実装です.

これは正答数が一つの場合ではありますが,言いたいことは引数のハンドリングは複雑になればなるほどミスしやすくなり,その結果,意図しない動作を引き起こすことにつながる,ということです.

問題形式が複数あるんだよ,ということですが,この構造体形式が違うようにできればフォームの呼び出しメソッドをオーバーライドすることによって問題形式の多様化にも柔軟に対応できると思います(フォームのデザインはオーバーライドされている個々のメソッドで必要なパネルを引っ張りだしたりする).

とりとめのない文章になってしまって申し訳ないのですが,ニュアンスを汲んでいただけると幸いです.

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/08 17:52

    引数11個は失笑、そういっていただけてよかったです。

    結局これはこのままいきで進行してます。

    回答がロジックとGUIがセットなので、単にロジックだけ切り出しても、シンプルにならない、というのが最大の理由です。

    キャンセル

  • 2015/12/08 19:27

    そうですね,プログラマの格言として「動いているものはいじらない」というのがあります.
    今後は自分が実装した時に満足するコーディングができればいいんじゃないでしょうかね?
    (今のコーディングを不本意と思っているのはコメントの端々から伝わってきます)

    キャンセル

0

提示されれた情報だけではわかりませんが、ひとつのまとまった機能は1ヶ所で定義してそれを共有するのが王道だと思います。システムの調査を依頼されて、大企業でも消費税率をひとつのfunctionを作って共有しようとしないところはびっくりしました。わたしが調査できたのはごく一部でしたが、たぶん作ったサブシステムか個人毎に作っていたのかも?

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/11/30 13:13

    なるほど。

    わたしもまとめるのが王道だと思うのです。

    とはいえ、という感じで。。。。

    キャンセル

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

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