none
コードの整理2回目・・・ RRS feed

  • 質問

  • trapemiya様へ
    こんにちは、剣道です。
    "遅れ表示時間画面"のPGを修正しました、

    trapemiya様から教えていただいた方法で、
    コードが大幅に削減しまして、予想の通り動きました。
    大変勉強になりました、本当に有難う御座いました。

     

    申し訳ございませんが、まだ、入力のチェック項目を増やしました。
    「開始時間」textboxに入力があるレコードは、有効データ。
    「開始時間」textboxに入力がないレコードは、無効データ。
    有効データの矛盾チェックを行います。"たすきがけ"

    例:有効データ0,1,2,3,7行の場合:
    mapCd1FromTextBox  =  mapCd0ToTextBox
    mapCd2FromTextBox  =  mapCd1ToTextBox
    mapCd3FromTextBox  =  mapCd2ToTextBox
    mapCd7FromTextBox  =  mapCd3ToTextBox

    或は例:有効データ1,3,7行の場合:
    mapCd1FromTextBox  =  mapCd0ToTextBox
    mapCd3FromTextBox  =  mapCd1ToTextBox
    mapCd7FromTextBox  =  mapCd3ToTextBox

     

    今、一行ずつチェックしています、動きましたが、
    修正しないといけないところが結構あると思いますが、

    申し訳ございません、ご時間が有る時、ご指摘お願いできますか?
    一言でも、よろしくお願いいたします。

    Code Snippet

    private void adaptButton_Click(object sender, EventArgs e)
            {
                bool bootb1 = false;
                bool bootb2 = false;
                bool bootb3 = false;
                bool bootb4 = false;
                bool bootb5 = false;
                bool bootb6 = false;

                TextBox tb1 = new TextBox();
                TextBox tb2 = new TextBox();
                TextBox tb3 = new TextBox();
                TextBox tb4 = new TextBox();
                TextBox tb5 = new TextBox();
                TextBox tb6 = new TextBox();
                TextBox tb7 = new TextBox();

                // 1行目と9行目の入力確認
                if (!CheckInput())
                {
                    MessageBox.Show("1行目或は9行目の入力項目エラーです。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                    return;
                }
                if (!CheckTime(mapCd1FromTextBox, mapCd1ToTextBox, mapCd1UnitTextBox, calcMod1ComboBox, mapCd0ToTextBox, mapCd1FromTextBox))
                {               
                    return;
                }

                //一行目******************
                if (effice)
                {
                    bootb1 = true;
                    tb1 = mapCd1ToTextBox;
                }
                else
                {
                    tb1 = mapCd0ToTextBox;
                }
                if (!CheckTime(mapCd2FromTextBox, mapCd2ToTextBox, mapCd2UnitTextBox, calcMod2ComboBox, tb1, mapCd2FromTextBox))
                {
                    return;
                }

                //二行目*************************
                if (effice)
                {
                    bootb2 = true;
                    tb2 = mapCd2ToTextBox;
                }
                else
                    if (bootb1)
                    {
                        tb2 = mapCd1ToTextBox;
                    }
                    else
                    {
                        tb2 = mapCd0ToTextBox;
                    }

                if (!CheckTime(mapCd3FromTextBox, mapCd3ToTextBox, mapCd3UnitTextBox, calcMod3ComboBox, tb2, mapCd3FromTextBox))           
                {
                    return;
                }

                //三行目*************************
                if (effice)
                {
                    bootb3 = true;
                    tb3 = mapCd3ToTextBox;
                }
                else if (bootb2)
                {
                    tb3 = mapCd2ToTextBox;
                }
                else if (bootb1)
                {
                    tb3 = mapCd1ToTextBox;
                }
                else
                {
                    tb3 = mapCd0ToTextBox;
                }
                if (!CheckTime(mapCd4FromTextBox, mapCd4ToTextBox, mapCd4UnitTextBox, calcMod4ComboBox, tb3, mapCd4FromTextBox))           
                {
                    return;
                }

                //四行目*************************
                if (effice)
                {
                    bootb4 = true;
                    tb4 = mapCd4ToTextBox;
                }
                else if(bootb3)
                {
                    tb4 = mapCd3ToTextBox;
                }
                else if (bootb2)
                {
                    tb4 = mapCd2ToTextBox;
                }
                else if (bootb1)
                {
                    tb4 = mapCd1ToTextBox;
                }
                else
                {
                    tb4 = mapCd0ToTextBox;
                }
                if (!CheckTime(mapCd5FromTextBox, mapCd5ToTextBox, mapCd5UnitTextBox, calcMod5ComboBox, tb4, mapCd5FromTextBox))           
                {
                    return;
                }
                //五行目*************************
                if (effice)
                {
                    bootb5 = true;
                    tb5 = mapCd5ToTextBox;
                }
                else if (bootb4)
                {
                    tb5 = mapCd4ToTextBox;
                }
                else if (bootb3)
                {
                    tb5 = mapCd3ToTextBox;
                }
                else if (bootb2)
                {
                    tb5 = mapCd2ToTextBox;
                }
                else if (bootb1)
                {
                    tb5 = mapCd1ToTextBox;
                }
                else
                {
                    tb5 = mapCd0ToTextBox;
                }
                if (!CheckTime(mapCd6FromTextBox, mapCd6ToTextBox, mapCd6UnitTextBox, calcMod6ComboBox, tb5, mapCd6FromTextBox))           
                {
                    return;
                }

                //六行目*************************
                if (effice)
                {
                    bootb6 = true;
                    tb6 = mapCd6ToTextBox;
                }
                else if (bootb5)
                {
                    tb6 = mapCd5ToTextBox;
                }
                else if (bootb4)
                {
                    tb6 = mapCd4ToTextBox;
                }
                else if (bootb3)
                {
                    tb6 = mapCd3ToTextBox;
                }
                else if (bootb2)
                {
                    tb6 = mapCd2ToTextBox;
                }
                else if (bootb1)
                {
                    tb6 = mapCd1ToTextBox;
                }
                else
                {
                    tb6 = mapCd0ToTextBox;
                }
                if (!CheckTime(mapCd7FromTextBox, mapCd7ToTextBox, mapCd7UnitTextBox, calcMod7ComboBox, tb6, mapCd7FromTextBox))
                {
                    return;
                }
                //七行目***************************
                if (effice)
                {
                    tb7 = mapCd7ToTextBox;
                }
                else if (bootb6)
                {
                    tb7 = mapCd6ToTextBox;
                }
                else if (bootb5)
                {
                    tb7 = mapCd5ToTextBox;
                }
                else if (bootb4)
                {
                    tb7 = mapCd4ToTextBox;
                }
                else if (bootb3)
                {
                    tb7 = mapCd3ToTextBox;
                }
                else if (bootb2)
                {
                    tb7 = mapCd2ToTextBox;
                }
                else if (bootb1)
                {
                    tb7 = mapCd1ToTextBox;
                }
                else
                {
                    tb7 = mapCd0ToTextBox;
                }
                // 八行目
                if (mapCd8FromTextBox.Text != tb7.Text)
                {
                    MessageBox.Show("入力エラー、開始時間と前行の終了時間が等しくないです。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                    return;
                }

                DialogResult result = MessageBox.Show("更新してもよろしいですか?",・・・・・・・・・・・・・・省略

    ****************************************

            /// <param name="fromTb">開始時間</param>
            /// <param name="ToTb">終了時間</param>
            /// <param name="unitTb">表示時間</param>
            /// <param name="calcCb">計算方法</param>
            /// <param name="equalToTb">有効データ終了時間(比較)</param>
            /// <param name="equalFromTb">有効データ開始時間(比較)</param>
            /// <returns>成否結果</returns>
            private bool CheckTime(TextBox fromTb, TextBox ToTb, TextBox unitTb, ComboBox calcCb,TextBox equalToTb,TextBox equalFromTb)
            {
                if (fromTb.Text.Trim().Length != 0)
                {
                    if (equalToTb.Text == equalFromTb.Text)
                    {
                        if (ToTb.Text.Trim().Length != 0)
                        {
                            if (int.Parse(fromTb.Text) < int.Parse(ToTb.Text))
                            {
                                if (unitTb.Text.Trim().Length != 0)
                                {
                                    if (calcCb.Text.Trim().Length != 0)
                                    {
                                        effice = true;
                                        return true;
                                    }
                                    MessageBox.Show("入力エラー、計算方法が未選択です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                                    return false;
                                }
                                MessageBox.Show("入力エラー、表示単位時間が未入力です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                                return false;
                            }
                            MessageBox.Show("入力エラー、終了時間は開始時間より小さく或は等しくなりました。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                            return false;
                        }
                        MessageBox.Show("入力エラー、終了時間が未入力です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                        return false;
                    }
                    MessageBox.Show("入力エラー、開始時間と前行の終了時間が等しくないです。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                    return false;
                }
                effice = false;
                return true;
            }      

     

     

    2008年3月13日 9:25

回答

  •  剣道 さんからの引用

    申し訳ございません、ご時間が有る時、ご指摘お願いできますか?
    一言でも、よろしくお願いいたします。

    もちろん、私もできるだけ力になりたいと思っていますが、この掲示板にはレベルの高い人がたくさんいらっしゃいますから、私個人宛ではなくて、広く回答を求めていることがわかるような書き方が良いと思いますよ。いろんな意見が出ればそれだけ知識が広がると思います。

     

    さて、たすきがけのチェックを追加されたのですね。
    考え方として、いろんなチェックを一度にするのではなく、チェック毎に切り分けていくとプログラムが見通しが良くなります。
    今回は、1行が正しく入力されているか?ということと、正しくたすきがけになっているか?の2種類のチェックがあります。私はこのような場合、横方向のチェックを横チェック、縦方向のチェックを縦チェックと大きく2つに分けてエラーチェックを行います。

    今回の場合は、縦チェックをするためには横チェックが全て通っている必要がありますから、まず横チェックを行い、それが全てパスすれば縦チェックするようにすれば良いと思います。

     

    縦チェックをするには配列にデータを入れてしまった方が楽です。一旦、配列にデータを全て入れてしまいましょう。
    あくまで一例ですが、テキストボックスとコンボボックスをある規則で名前付けすれば、以下のようなコードで配列化することができます。

    3行 x 4列 のコントロールがある。左3列がテキストボックス、残りの右端がコンボボックスである。

    テキストボックスの名前付け規則は、textBox99であり、テキストボックスの名前付け規則は、comboBox99である。いずれも99は数字2桁を表し、3行 x  4列のマトリックスにおいて、以下のようになっている。

    textBox00   textBox01   textBox02   comboBox03

    textBox10   textBox11   textBox12   comboBox13

    textBox20   textBox21   textBox22   comboBox23

     

    string[,] arrayControl = new string[3, 4];
       
    foreach (Control control in this.Controls)
    {
       if (control.Name.Substring(0, 7) == "textBox" || control.Name.Substring(0, 8) == "comboBox")
       {
          string indexStr = control.Name.Substring(control.Name.Length - 2, 2);
          arrayControl[int.Parse(indexStr.Substring(0, 1)), int.Parse(indexStr.Substring(1, 1))]
                                                       = control.Text;
       }
    }

     

    配列にしてしまえば簡単で、配列を利用して横チェック、縦チェックをしましょう。
    横チェックは前回私が提示したCheckTimeを用いれば良いですし、縦チェックは以下のようにすれば良いでしょう。

     

    //縦チェック たすきがけのチェック

    string toTime_save = string.Empty;

     

    for (int i = 0; i < arrayControl.GetLength(0); i++)
    {
       if (arrayControl[i, 0] != string.Empty && toTime_save != string.Empty)
          if (arrayControl[i, 0] != toTime_save)
             MessageBox.Show("error! fromとtoが一致せず。" + toTime_save + "->" + arrayControl[i, 0]);

     

       if (arrayControl[i, 1] != string.Empty)
          toTime_save = arrayControl[i, 1];
    }

     

    #arrayControlの名前が適切ではないですね。途中で設計を変えたものですから(^^;

    2008年3月13日 16:15
    モデレータ

すべての返信

  • やりたいことがよくわかりません…

     

    なんとなくですが、配列を使ったりしてもうちょっと一般化しましょう。

    剣道さんの書き方だと10000通りあるものだと本当に10000通り書いてしまいそうで怖いです。

    2008年3月13日 15:20
  •  剣道 さんからの引用

    申し訳ございません、ご時間が有る時、ご指摘お願いできますか?
    一言でも、よろしくお願いいたします。

    もちろん、私もできるだけ力になりたいと思っていますが、この掲示板にはレベルの高い人がたくさんいらっしゃいますから、私個人宛ではなくて、広く回答を求めていることがわかるような書き方が良いと思いますよ。いろんな意見が出ればそれだけ知識が広がると思います。

     

    さて、たすきがけのチェックを追加されたのですね。
    考え方として、いろんなチェックを一度にするのではなく、チェック毎に切り分けていくとプログラムが見通しが良くなります。
    今回は、1行が正しく入力されているか?ということと、正しくたすきがけになっているか?の2種類のチェックがあります。私はこのような場合、横方向のチェックを横チェック、縦方向のチェックを縦チェックと大きく2つに分けてエラーチェックを行います。

    今回の場合は、縦チェックをするためには横チェックが全て通っている必要がありますから、まず横チェックを行い、それが全てパスすれば縦チェックするようにすれば良いと思います。

     

    縦チェックをするには配列にデータを入れてしまった方が楽です。一旦、配列にデータを全て入れてしまいましょう。
    あくまで一例ですが、テキストボックスとコンボボックスをある規則で名前付けすれば、以下のようなコードで配列化することができます。

    3行 x 4列 のコントロールがある。左3列がテキストボックス、残りの右端がコンボボックスである。

    テキストボックスの名前付け規則は、textBox99であり、テキストボックスの名前付け規則は、comboBox99である。いずれも99は数字2桁を表し、3行 x  4列のマトリックスにおいて、以下のようになっている。

    textBox00   textBox01   textBox02   comboBox03

    textBox10   textBox11   textBox12   comboBox13

    textBox20   textBox21   textBox22   comboBox23

     

    string[,] arrayControl = new string[3, 4];
       
    foreach (Control control in this.Controls)
    {
       if (control.Name.Substring(0, 7) == "textBox" || control.Name.Substring(0, 8) == "comboBox")
       {
          string indexStr = control.Name.Substring(control.Name.Length - 2, 2);
          arrayControl[int.Parse(indexStr.Substring(0, 1)), int.Parse(indexStr.Substring(1, 1))]
                                                       = control.Text;
       }
    }

     

    配列にしてしまえば簡単で、配列を利用して横チェック、縦チェックをしましょう。
    横チェックは前回私が提示したCheckTimeを用いれば良いですし、縦チェックは以下のようにすれば良いでしょう。

     

    //縦チェック たすきがけのチェック

    string toTime_save = string.Empty;

     

    for (int i = 0; i < arrayControl.GetLength(0); i++)
    {
       if (arrayControl[i, 0] != string.Empty && toTime_save != string.Empty)
          if (arrayControl[i, 0] != toTime_save)
             MessageBox.Show("error! fromとtoが一致せず。" + toTime_save + "->" + arrayControl[i, 0]);

     

       if (arrayControl[i, 1] != string.Empty)
          toTime_save = arrayControl[i, 1];
    }

     

    #arrayControlの名前が適切ではないですね。途中で設計を変えたものですから(^^;

    2008年3月13日 16:15
    モデレータ
  • trapemiya様、+かずき+様へ
    たくさんのことを教えていただき、大変有難うございます。

    これから、最初の実行手順の理解にもっと努めていきたいと思っています。
    (投稿のしかたも気を付けます。)


    本当に習ってきた感覚です。

    +かずき+様の仰った通り、私の書き方なら、本当に怖いです。
    出来るまで沢山の訓練をしないといけないです。
    ご指導、本当に有難うございました。
    また、よろしくお願いいたします。

    2008年3月14日 3:07