QA@IT

Dictionary クラスは追加と上書きのメソッドを別にした設計が良いですか?

6000 PV
IDictionary<int, string> dic = new Dictionary<int, string>();
for (int i = 0; i < 10; i++)
{
    dic[i * i] = "abc"; // ←例題として、キーは整数の2乗とする。
}

// ↓既存の dic[49] のエントリーに対して上書きしようとして、キーを間違えて 47 にした。
dic[47] = "xyz"; // ←エラーにならず、新規のエントリーの追加になってしまう。

// ↓追加であり上書きではないことを明示する記法はある。
dic.Add(47, "xyz");

// ↓上書きであり追加ではないことを明示する記法がほしい。
//dic.Update(47, "xyz");

上記のサンプルコードのコメントでほぼ言い尽くしているのですが補足します。

Dictionary クラスに対して同じキーで上書きしたいときは、プロパティー(dic[x])を使えばできるのですが、これだとまだ存在しないキーのエントリーの追加もできてしまいます。
追加だけしたくて上書きをしたくないときは Add メソッドを使えば良いのですが、その逆に上書きだけしたくて追加をしたくないときに使えるメソッドが Dictionary クラスには存在しません。上記のサンプルコードに書いたように、架空の Update メソッドのようなものがあれば便利だと思います。そしてプロパティー(dic[x])では上書きだけ許し、追加はできないように制限したほうが良いと思います。

ここでは例として Dictionary クラスを挙げましたが、似たようなことを感じるクラスはほかにもいろいろあります。話を一般化すると、とくに追加・上書きという機能に限らず、1つのメソッドはできるだけ単純な機能に絞り、複合させた機能は持たせないようにしたほうが良いという考えが、背景にあります。

主観的になりがちな話題だと思いますが、「○○というメリット・デメリットがあるので、メソッドを別にしたほうが良い・別にしないほうが良い」というご意見をいただければ幸いです。

  • 質問のタイトルは質問形式(質問文)でお願いします。 -
  • 当初、質問タイトルに「たとえば」という言葉を前置することで、Dictionary クラスに過度に特化した質問ではないことを明示していましたが、質問タイトルを変更したのでここにその趣旨を明記します。 -
  • 質問文が編集された場合、編集が有ったことを知る手段として質問文の下部に表示される「履歴 (3)」のようなカウンターの増加を注意深く監視する必要があります。編集が有ったことを知っていれば、履歴を見るだけで良いですが、編集が有ったことを知らなかった場合にカウンターの増加を見逃せば、編集が有ったことも見逃す恐れがあります。 -
  • 「私の質問は、その自前で用意するクラスはどう設計すれば良いものになるか、ということでした。」ということなら、今回の質問の内容が根本的なところから大幅に変わってしまいます。もはや、何を質問したいのか、その主旨がさっぱり見えてきません。もし私に十分な権限があるなら、この質問に-1を付けます。 -
  • 「大幅に変わってしまいます」というご批判がありましたが、どう変わっているのでしょうか?どう大幅なのでしょうか?
    私は後出しはしていません。最初から Dictionary は例だと言っています。Dictionary クラスに過度に特化した質問ではないことを言っています。
    -
  • Dictionary を引き合いに出しているので話が分からないとおっしゃるかたから、自前でクラスを用意するほうが良いと助言されたのであれば、もともと私もそれほど Dictionary に特化したつもりはなかったので、質問から Dictionary の例を取り除けば、質問として残ることはそういった自前で用意するクラスのことになる、と返答することになるのは当然のことです。 -
  • えーと、ようするにこれは、「オブジェクトはどのように設計するのが良いのでしょうか?」という質問なんですか? 表題にあるとおりDictionaryクラスというものがあったから話の方向性が見えていましたが、この質問からDictionaryクラスの話を除いたら、内容が漠然としすぎて答えようがありません。これは大きな違いだと思います。 -
  • その「自前のクラス」がDictionaryと同じようなものならば、あくまでもDictionaryクラスで話を進めた方がいいと思います。自前のクラスの話を持ち出す必要はありません。もし、それがDictionaryと異なるものならば、どんな仕様のクラスなのか提示してください。 -
  • これは「オブジェクトはどのように設計するのが良いのでしょうか?」という質問です。
    ここで確認させてください。stripeさんは、私の 2012/09/24 19:43 のノートは読まれたのでしょうか?
    もしも読まれたのであれば、stripeさんはそれに対する見解を示されないまま、今までの経緯は無視して新たに「内容が漠然としすぎて」というご批判をされていることになります。
    -
  • 新しいご批判も、ひとつのご批判として読ませてはいただきますが、そのご批判にはとくに根拠はないのですよね?
    stripeさんはDictionaryクラスで話を進めた方がいいと思われている、というだけが根拠ですね?それでよろしいですか?
    -
  • 私が言いたいのは、「質問の内容が分りずらいため答えにくいので、質問内容をはっきりさせてください」ということです。私がDictionaryで話を進めたいのではなく、unibonさんがどう話を進めたいのかが知りたいのです。質問内容が分らないためunibonさんにいろいろ話を聞いていますが、聞けば聞くほど内容が漠然としてきます。 -
  • 通常は、質問の主旨などを本題とし、捕捉として「例えば、Dictionaryクラスにについて言うならば・・・」となるところですが、unibonさんの質問は例え話が本題になっているように見えます。質問のタイトルはknsmrさんの指摘により修正されましたが、あれは表面的な語句修正ではなく、「例え話を質問の本題にしないでくれ」という意味だろうと思います。 -
  • まさに、「オブジェクトはどのように設計するのが良いのでしょうか?」という質問こそが、内容が漠然としすぎた質問なのです。unibonさんは最初から漠然とした質問がしたかったのでしょうか?もしも、これが質問の主旨だと言うならば、そのようにタイトルを修正した方が良いでしょう。ただし、漠然とした回答しか来ないと思います。 -
  • 私は批判されている側なので、批判する人に対し批判の根拠を聞く権利があると思います。再度確認します。stripeさんは、私の 2012/09/24 19:43 のノートは読まれたのですよね?「大幅に変わってしまいます」というご批判は今も継続されているのですか?それとも取り下げられたのですか?これが「内容が漠然としすぎて」という批判の根拠として使われているのかいないのかは重要なことです。 -
  • もちろん、そのノートを読んでいます。内容が漠然としていることの根拠は何度も説明しています。今回の質問の中では、Dictionaryクラスの仕様が、回答に繋がる唯一の具体的な情報で、私も他の人達もその仕様に沿った形で回答を付けてると思います。それなのに、「Dictionaryはあくまでも例え話ですから」などと言われると、話が振り出しに戻ったような気分になります。 -
  • もちろん今回の質問者はunibonさんで、その質問者が「Dictionaryはあくまでも例え話だ」と言うなら、そのとおりなのでしょう。回答者側である私が、質問内容を読み違えたと言われてもしかたありません。それはそれとして、unibonさんにはもう少し質問内容の説明をお願いしたいと思います。 -
  • 私は批判されている側なので、批判する人に対し批判の根拠を聞く権利があると思います。再度確認します。stripeさんは、「大幅に変わってしまいます」というご批判は今も継続されているのですか?それとも取り下げられたのですか?これが「内容が漠然としすぎて」という批判の根拠として使われているのかいないのかは重要なことです。 -
  • 補足します。stripeさんの「話が振り出しに戻ったような気分になります」というご説明からは、「Dictionaryはあくまでも例え話ですから」という主張がまるで後出しであるかのように聞こえます。
    私は後出しではないと前から主張していますので、それを確認するためには、まだstripeさんからご返答をいただいていない質問を繰り返さざるをえません。
    -
  • 「大幅に変わってしまう」件については、今も継続しています。今回の質問の中でのDictionaryクラスの位置づけは、重要だと思います。 -
  • 私は批判されている側なので、批判する人に対し批判の内容を聞く権利があると思います。再度確認します。
    stripeさんから、「大幅に変わってしまいます」というご批判がありましたが、どう変わっているのでしょうか?どう大幅なのでしょうか?
    私は後出しはしていません。最初から Dictionary は例だと言っています。Dictionary クラスに過度に特化した質問ではないことを言っています。
    -
  • 不毛なやりとりが続いているような気がしますので、unibonさんが仕切り直しDictionaryの事を先に出さずに一番聞きたいことを先に書きデータ例、規則例を記述し実装例を書かれた方が良いかと思います。 -
  • 実装例が先にきてその中にはDictionaryだけで自前のクラスがないので話が大幅に変わるということを言っているのではないでしょうか?(あくまで用意されたクラスに対する論評をしたいのか、自分で作るクラスに対する基準を考えたいのかが分からないと思う) -
  • unibonさんは同じことを繰り返すばかりで話が進みません。ところで、私から質問内容の説明をお願いしてるところですが、その件はどうなったのでしょうか?もし、説明が難しいようなら、代わりにDictionary以外の例を幾つか上げてもらうだけで構いません。 -
  • ここにまとめて書かせていただきますが、みなさまのご回答はすべて拝見させていただいております。
    質問者としてお礼申し上げます。ありがとうございます。
    -
  • そろそろですね。質問ガイドラインをお読みの上で、思われたとおりに評価をお願いいたします。 -

回答

ここでは例として Dictionary クラスを挙げましたが、似たようなことを感じるクラスはほかにもいろいろあります。話を一般化すると、とくに追加・上書きという機能に限らず、1つのメソッドはできるだけ単純な機能に絞り、複合させた機能は持たせないようにしたほうが良いという考えが、背景にあります。

むしろ、既に登録されているかテストしてからになるので、
上書き不可なメソッドの方が復号させた機能ではありませんか?

もちろん、そういう要件(上書き不可)があって、それに応じた拡張がされるのは必然だと思います。
その場合でもUpdateメソッドの方に、上書きする対象が無ければ例外を生成するか、DB操作のように、Updateメソッドの戻り値が更新したキーの数になるようにした方がピンと来ますが。

ここから蛇足
そもそもdic[7*7]="xyz"とすべきとこをdic[6*6]="xyz"としたら救われないじゃないですか。
そういうのは、単体試験で確認すべき事ではないでしょうか?
仮に、上書き不可な実装だったとして、
dic[47] = "xyz";
で例外が発生するのはdicの試験であって、
dicを使う側の試験ではありませんよ。

編集 履歴 (0)

結論が出ないまま終わるのも残念なので
unibonさんの提示した内容に合うと思われるコードを書いてみました。

1つ目がkey検査付きの基本となるクラス。
2つ目が今回の提示されたキーチェック条件を実装したもの。
3つ目がFormによる呼び出し例で2つのテキストボックスにkeyと値を入れ
Exceptionは派生クラスを作成しています。
Add優先の処理、Update優先の処理をそれぞれ行うものです。
AddなのかUpdateなのかはチェックしてみないと分からないのでこのような処理で
試す必要があるかと思い2通り用意してみました。Addを呼ぶかUpdateを呼ぶかに
関わらず間違ったメソッドを呼ぶと例外が発生してしまい効率が悪い気がしました。
中身を見ずにAddをするかUpdateをするか決められるのなら間違い発生時に例外と
なるので早期に修正することが出来るかとは思います。提示されたような最初に有効な
キーをAddしてしまえる固定キーのパターンでは有効かもしれません。

    public class DicWithKeyCheck<Tkey, TValue>:IEnumerable<KeyValuePair<Tkey,TValue>>
    {
        private Dictionary<Tkey,TValue> _dic;

        public DicWithKeyCheck()
        {
            _dic = new Dictionary<Tkey, TValue>();
        }

        virtual public Boolean CheckKey(Tkey key)
        {
            return true;
        }

        public void Add(Tkey key, TValue value)
        {
            if (CheckKey(key))
            {
                if (!_dic.ContainsKey(key))
                {
                    _dic.Add(key, value);
                }
                else
                {
                    throw new KeyExistsException("keyが既に存在します。");
                }
            }
            else
            {
                throw new KeyInvalidException("Keyが不正です。");
            }
        }

        public void Update(Tkey key, TValue value)
        {
            if (CheckKey(key))
            {
                if (_dic.ContainsKey(key))
                {
                    _dic[key] = value;
                }
                else
                {
                    throw new KeyNotFoundException("keyが追加されていません。");
                }
            }
            else
            {
                throw new KeyInvalidException("Keyが不正です。");
            }           
        }

        IEnumerator<KeyValuePair<Tkey, TValue>> IEnumerable<KeyValuePair<Tkey, TValue>>.GetEnumerator()
        {
            return _dic.GetEnumerator();
        }

        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
        {
            return _dic.GetEnumerator();
        }
    }
    class SampDicWithKeyCheck:DicWithKeyCheck<int,string>
    {
        public SampDicWithKeyCheck():base()
        {
        }

        public override bool CheckKey(int key)
        {
            int i1 = (int)Math.Floor(Math.Sqrt(key));
            if (i1 * i1 == key)
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }
    public partial class Form1 : Form
    {
        private SampDicWithKeyCheck _sampdic;

        public Form1()
        {
            InitializeComponent();
            _sampdic = new SampDicWithKeyCheck();
        }


        private void button1_Click(object sender, EventArgs e)
        {
            //--- Add先の場合
            int key = int.Parse(textBox1.Text);
            string value = textBox2.Text;
            try
            {
                _sampdic.Add(key, value);
            }
            catch (KeyExistsException)
            {
                _sampdic.Update(key, value);
            }
        }

        private void button2_Click(object sender, EventArgs e)
        {
            //--- Update先の場合
            int key = int.Parse(textBox1.Text);
            string value = textBox2.Text;
            try
            {
                _sampdic.Update(key, value);
            }
            catch (KeyNotFoundException)
            {
                _sampdic.Add(key, value);
            }
        }

        private void button3_Click(object sender, EventArgs e)
        {
            listBox1.Items.Clear();
            foreach(var kv in _sampdic)
            {
                listBox1.Items.Add(String.Format("key:{0} , value:{1}", kv.Key, kv.Value));
            }
        }


    }
編集 履歴 (0)

Dictionaryの性質はkeyによるデータ管理であり、
key自体の間違いを検出するためのものではありません。
なのでinstance[]の性質を除いてしまったらそれはDictionaryとは
いえないのではないでしょうか?
instance[key]=value
の形でkeyに対するvalueの設定が設定済であるかどうかにかかわらず
保証されるのは一般的には便利ではないかと思います。
AddするのかUpdateするのか分からないときにkeyの存在を確認して
AddまたはUpdateを呼ぶ分岐をそのたびに記述しなくて良いのは
Dictionaryの性質からはすごく適していると思います。
keyの間違いというのはプロジェクト固有の性質であり(keyの間違い自体は
多くのプロジェクトで存在しますがそれは共通の間違いなわけではない)一般的な
性質ではないと思います。なので固有の機能を実装するなら新しい
クラスを作るべきであって既存のクラスにその性質を求めるべきでは
ないと思います。AddとUpdateと作ったとしてもどっちを呼ぶかは
keyの正当性チェックをしてdictionary内にkeyが存在しているか
確認して存在しなければAdd,存在すればUpdateというロジックが
多数発生してしまうのではないでしょうか?
例題として提示されたコードではあらかじめ
0*0 ~ 9*9までのキーを設定していますが、keyとして正当なものが有限でなかったら
これは出来ません。整数の二乗というのが正当なkeyの条件なので与えられたkeyに
対してそれが整数の二乗であるかをまずチェックしなければdicにkeyがないからといって
それが不正なkeyとは言えないです。例えば121がkeyとして渡されてもこれは正当なkey
でなければならないのにdicにないため不正なkeyという間違ったチェックが行われてしまいます。
このようにdictionaryにkeyのチェックを行わせるのはテストを省けるほど確実なことには
ならないです。

編集 履歴 (0)
  • ご回答ありがとうございます。まずテストについては、私がこの質問をするのは、テストを省きたいからではないです。必要なテストはします。テストをすれば、極端な話、Dictionary を使わずにすべて配列で処理していても、オブジェクト指向言語を使わなくても、正しく動くコードは完成させることができます。ただし、それに到達するまでの時間や工数がかかってしまいます。私の質問の目的はそれを削減することです。 -
  • しかし、49を間違えて47を渡した時に、メソッドが別ならばすぐに例外が起きてバグを見つけられたのに、メソッドが別になっていないと例外が起きません。しかしテスト結果は正しくなく、なにか動きがおかしいので、それをデバッガーを使って探らないといけなくなり、Dictionary の要素を見てようやく、変な要素が追加されていることが分かり、バグの発見に時間がかかります。 -
  • オブジェクト指向でよく言われる、いわゆる "Tell, Don't Ask" を重視するならば、instance[key]=value
    で追加と上書きを兼用することは、クラスに状態を尋ねていないので良いという考えもできると思います。しかし、そうであれば ContainsKey という尋ねるメソッドが別途存在しているのは、高レベルのAPIと低レベルのAPIが混在して好ましくないとも言えます。
    -
  • 私は、高レベルと低レベルが混在するよりは、高レベルだけで統一するか低レベルだけで統一するかのどちらかにしたほうが良いと考えます。そしておそらく高レベルだけで統一することは難しいと思いますので、残るやりかたである低レベルだけで統一したほうが良いと考えます。なおそのために、keyの存在を確認することは、しかたがないと考えます。 -
  • Dictionaryを引き合いに出しているので話がいまひとつよく分からないのですが、プロジェクト内の仕様に合うクラスはやはり自前で用意されるのが良いのではないでしょうか? -
  • 今QAの内容を後からみても進行も結論もわかりにくい気がします。(内容がQA向きではない気がする) -
  • 私の質問は、その自前で用意するクラスはどう設計すれば良いものになるか、ということでした。Dictionary は質問を具体的にするための例です。現時点でのQA@ITの質問ガイドラインには「ITエンジニアが関係する質問なら何でもどうぞ」と記載されています。ありがとうございました。 -

サンプルコードの中に、「49と47を間違えて...」といったことが書かれていますが、
今回の話は、ソースコードの誤入力防止の話なのでしょうか?

そのサンプルコードのように、キーは整数の2乗であるという仕様ならば、
dic[49]の代わりにdic[7*7]と書くか、定数を定義してdic[fortyNine]と書くと思います。

また、含まれるキーの内容が固定的に決っているなら、Dictionaryを使わずに独自の構造体を定義するなどすると思います。

その他に、イミュータブルな(不変な)戦略でプログラムを組むという方法もあります。
これだと、追加も上書きも発生しませんから、根本的なレベルで解決します。議論にすらなりません。

今回の質問は、「updateメソッドにメリットやデメリットがあるか?」と言った内容になっていますが、それに対する私の答えは、「updateメソッドは不要」です。
特にデメリットがあるからという訳ではなく、それが無くても問題が解決できるからです。

しかし、もしunibonさんが、updateメソッドがどうしても必要となるような前提条件を設定するならば、そのときは前提条件により「updateメソッドは必要である」という結論になると思います。
Dictionaryのサブクラスを作って、updateメソッドを実装すれば問題は解決します。

編集 履歴 (1)
  • ご回答ありがとうございます。私が問題とするのは、突き詰めればソースコードの誤入力に対する対処です。ただし、おっしゃるような定数や Dictionary 以外のクラスを使ったほうが良ければ使った上で、それでもまだ Dictionary を使わざるを得ない場面で残りがちなバグをコーディング段階で残らないようにするための対処です。 -
  • さきほどコーディング段階と書きましたが、まずはテスト段階で意図しない追加を防ぎたい(追加になったら例外を出したい)です。そして、テスト段階で弾ければ、コーディング段階でもバグの減少につながるのではないか、と漠然と思っています。 -
  • あと、このような設計の話の場合は、すでに Dictionary クラスの仕様が存在しているため、それをどう改造すべきか(あるいは改造しないべきか)という話になりやすいです。それももちろんあって良いと思いますが、もしもゼロから Dictionary クラスを作るとしたらどう設計するのが良いか、という観点でのご意見もみなさまからいただけたらありがたいと思います。 -
  • 「テスト段階」ってどういう意味ですか?普通はコーディングしてからテストするのでは?テストファーストの場合でもコーディングしないとテストを実行出来ませんよね? -
  • サブクラスを作ってそれに Update メソッドを持たせたとしても、親クラスが持つメソッドへのアクセスを抑制することはできないので、うっかりコーディングで Update メソッドを呼ばずに dic[x] にアクセスして、更新するつもりで追加してしまうことが起こりえてしまいます。これを避けるためには、親クラスの段階で追加と更新のメソッドを別にするしかないと考えています。 -
  • 私が書いた「まずはテスト段階」とは、バグを発見する段階は、たぶんコーディングしたコードを眺めて発見するよりは、テストでコードを実行時に発見することがきっかけとしては先だろうな、という意味です。開発の順序は、おっしゃるようにコーディング→テストです。 -
  • コーディングしたコードを眺めてバグを発見しようとする場合も、たとえば変な要素が追加されていることがテストで分かっていたのならば、コード内で使われている追加のメソッドを重点的に調べれば良く、上書きのメソッドはそれより優先度は下げて良いはずです。しかし、追加と上書きのメソッドが分かれていないと、眺める対象が無駄に増えてしまいます。目視に限らずIDEでシンボルの検索をするような場合も同様です。 -
  • ようするに、テストの結果がNGのとき、デバッガを使わずに、コードの不具合箇所をピンポイントで知りたい。ということですか? -
  • 「はい」か「いいえ」かで言えば「はい」です。
    デバッガーを使わないことにかならずしもこだわるわけではありませんが、コードを眺める時間が短時間でもバグの場所を容易に知ることができるならそういうコードにしたいです。そういうコードにするために、そのコード内で使うクラスの設計を良いものにしたいです。
    -
ウォッチ

この質問への回答やコメントをメールでお知らせします。