none
コードの整理・・・ RRS feed

  • 質問

  • こんにちは、初心者の剣道です。
    よろしくお願いいたします。

    昨日の質問と関係あり。【昨日の質問:SQL DBのUPDATEについて 】
    今、"時間遅れ設定画面"の配置が変更しまして、(TextBoxとComboBox二列を増やしました。)
    一行はテキストボックス三つと、コンボボックス一つになっています。
    全部は9行、4列です。
    fromTimeTextBox0 ~ toTimeTextBox0 (開始時間~終了時間) mapCd0UnitTextBox(分単位) calcMod0ComboBox(計算方法)
      ↓    ↓        ↓    ↓
    fromTimeTextBox8 ~ toTimeTextBox8 (開始時間~終了時間) mapCd8UnitTextBox(分単位) calcMod8ComboBox(計算方法)

    やりたいことは、 テキストボックスと、コンボボックスの入力チェックです。
    ① 行毎に、開始時間が""の場合、"更新"ボタンを押し、DB更新できるようにする。
    ② 行毎に、開始時間が""ではない場合、終了時間の値をチェック、
      終了時間は開始時間より、大きいなら、OKです。
      開始時間と終了時間両方nullではない場合、分単位の値をチェック、
      分単位の値nullではない場合、計算方法の文字をチェック、
      計算方法を選択された場合、OKです。
      "更新"ボタンを押し、DB更新できるようにする。
    現時点で、コードを書きましたが、動きは一応大丈夫ですが、
    きれいなコードではない、無駄な構造があると思います。
    修正の方法について、ヒントや助言を教えて頂きたいです。
    よろしくお願いいたします。

    下記コードです。(長くなりまして、申し訳ございません)
    チェックメソッド

    Code Snippet
    private bool CheckInput()
            {
                for (int i = 0; i < 8; i++)
                {
                    switch (i)
                    {
                        case 0:
                            if (mapCd0FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd0ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd0FromTextBox.Text) < int.Parse(mapCd0ToTextBox.Text))
                                    {                                   
                                        break;
                                    }
                                    MessageBox.Show("入力エラー、終了時間は開始時間より小さく或は等しくなりました。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                                    return false;
                                }
                                MessageBox.Show("入力エラー、終了時間が未入力です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
                                return false;
                            }
                            break;
                        case 1:
                            if (mapCd1FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd1ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd1FromTextBox.Text) < int.Parse(mapCd1ToTextBox.Text))
                                    {
                                        if (mapCd1UnitTextBox.Text.Trim().Length != 0)                                   
                                        {
                                            if (calcMod1ComboBox.Text.Trim().Length != 0)
                                            {                                           
                                                break;
                                            }
                                            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;                           
                            }                       
                            break;
                        case 2:
                            if (mapCd2FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd2ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd2FromTextBox.Text) < int.Parse(mapCd2ToTextBox.Text))
                                    {
                                        if (mapCd2UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod2ComboBox.Text.Trim().Length != 0)
                                            {                                           
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                        case 3:
                            if (mapCd3FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd3ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd3FromTextBox.Text) < int.Parse(mapCd3ToTextBox.Text))
                                    {
                                        if (mapCd3UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod3ComboBox.Text.Trim().Length != 0)
                                            {
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                        case 4:
                            if (mapCd4FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd4ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd4FromTextBox.Text) < int.Parse(mapCd4ToTextBox.Text))
                                    {
                                        if (mapCd4UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod4ComboBox.Text.Trim().Length != 0)
                                            {
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                        case 5:
                            if (mapCd5FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd5ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd5FromTextBox.Text) < int.Parse(mapCd5ToTextBox.Text))
                                    {
                                        if (mapCd5UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod5ComboBox.Text.Trim().Length != 0)
                                            {
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                        case 6:
                            if (mapCd6FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd6ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd6FromTextBox.Text) < int.Parse(mapCd6ToTextBox.Text))
                                    {
                                        if (mapCd6UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod6ComboBox.Text.Trim().Length != 0)
                                            {
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                        case 7:
                            if (mapCd7FromTextBox.Text.Trim().Length != 0)
                            {
                                if (mapCd7ToTextBox.Text.Trim().Length != 0)
                                {
                                    if (int.Parse(mapCd7FromTextBox.Text) < int.Parse(mapCd7ToTextBox.Text))
                                    {
                                        if (mapCd7UnitTextBox.Text.Trim().Length != 0)
                                        {
                                            if (calcMod7ComboBox.Text.Trim().Length != 0)
                                            {
                                                break;
                                            }
                                            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;                           
                            }
                            break;
                    }               
                }
                return true;                 
            }

     

    2008年3月11日 13:09

回答

  • for (int i = 0; i < 8; i++)
    {
         switch (i)
         {
        case 0:
          ・
          ・
          ・
        case 7:

     

    とされていますが、forで回して順にcase句内が実行されるわけですから、ここの制御は意味がありません。べたに、

     

    if (mapCd0FromTextBox.Text.Trim().Length != 0)
    {
        if (mapCd0ToTextBox.Text.Trim().Length != 0)
        {
            if (int.Parse(mapCd0FromTextBox.Text) < int.Parse(mapCd0ToTextBox.Text))
            {                                   
                break;
            }
            MessageBox.Show("入力エラー、終了時間は開始時間より小さく或は等しくなりました。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
            return false;
        }
        MessageBox.Show("入力エラー、終了時間が未入力です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
        return false;
    }


    から


    if (mapCd7FromTextBox.Text.Trim().Length != 0)
    {
        if (mapCd7ToTextBox.Text.Trim().Length != 0)
        {
            if (int.Parse(mapCd7FromTextBox.Text) < int.Parse(mapCd7ToTextBox.Text))
            {
                if (mapCd7UnitTextBox.Text.Trim().Length != 0)
                {
                    if (calcMod7ComboBox.Text.Trim().Length != 0)
                    {
                        break;
                    }
                    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;                           
    }


    まで、素直に続けて書いた方がまだ良いです。

    素直に続けて書いたとして、同じ処理のコードが何度も出てきますから、その部分はサブルーチンにすることができます。例えば以下のような感じです。(コード未検証)


    CheckTime(mapCd0FromTextBox, mapCd0ToTextBox, mapCd0UnitTextBox, calcMod0ComboBox)
    CheckTime(mapCd1FromTextBox, mapCd1ToTextBox, mapCd1UnitTextBox, calcMod1ComboBox)
    CheckTime(mapCd2FromTextBox, mapCd2ToTextBox, mapCd2UnitTextBox, calcMod2ComboBox)
    CheckTime(mapCd3FromTextBox, mapCd3ToTextBox, mapCd3UnitTextBox, calcMod3ComboBox)
    CheckTime(mapCd4FromTextBox, mapCd4ToTextBox, mapCd4UnitTextBox, calcMod4ComboBox)
    CheckTime(mapCd5FromTextBox, mapCd5ToTextBox, mapCd5UnitTextBox, calcMod5ComboBox)
    CheckTime(mapCd6FromTextBox, mapCd6ToTextBox, mapCd6UnitTextBox, calcMod6ComboBox)
    CheckTime(mapCd7FromTextBox, mapCd7ToTextBox, mapCd7UnitTextBox, calcMod7ComboBox)


    private void CheckTime(TextBox fromTb, TextBox ToTb, TextBox unitTb, ComboBox cb)
    {
       errorMessage = string.Empty;

       if (fromTb.Text.Trim().Length != 0)
       {
           if (toTbText.Trim().Length != 0)
           {
               if (int.Parse(fromTb.Text) < int.Parse(toTbText))
               {
                   if (unitTb.Text.Trim().Length != 0)
                   {
                       if (cb.Text.Trim().Length != 0)
                       {
                           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;
       }
    }

    実際、まだ改善すべきところはたくさんありますが、一度に説明すると混乱されるといけませんので、少しずつ行きましょう。

    2008年3月11日 15:37
    モデレータ

すべての返信

  • for (int i = 0; i < 8; i++)
    {
         switch (i)
         {
        case 0:
          ・
          ・
          ・
        case 7:

     

    とされていますが、forで回して順にcase句内が実行されるわけですから、ここの制御は意味がありません。べたに、

     

    if (mapCd0FromTextBox.Text.Trim().Length != 0)
    {
        if (mapCd0ToTextBox.Text.Trim().Length != 0)
        {
            if (int.Parse(mapCd0FromTextBox.Text) < int.Parse(mapCd0ToTextBox.Text))
            {                                   
                break;
            }
            MessageBox.Show("入力エラー、終了時間は開始時間より小さく或は等しくなりました。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
            return false;
        }
        MessageBox.Show("入力エラー、終了時間が未入力です。", this.Name, MessageBoxButtons.OK, MessageBoxIcon.Error);
        return false;
    }


    から


    if (mapCd7FromTextBox.Text.Trim().Length != 0)
    {
        if (mapCd7ToTextBox.Text.Trim().Length != 0)
        {
            if (int.Parse(mapCd7FromTextBox.Text) < int.Parse(mapCd7ToTextBox.Text))
            {
                if (mapCd7UnitTextBox.Text.Trim().Length != 0)
                {
                    if (calcMod7ComboBox.Text.Trim().Length != 0)
                    {
                        break;
                    }
                    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;                           
    }


    まで、素直に続けて書いた方がまだ良いです。

    素直に続けて書いたとして、同じ処理のコードが何度も出てきますから、その部分はサブルーチンにすることができます。例えば以下のような感じです。(コード未検証)


    CheckTime(mapCd0FromTextBox, mapCd0ToTextBox, mapCd0UnitTextBox, calcMod0ComboBox)
    CheckTime(mapCd1FromTextBox, mapCd1ToTextBox, mapCd1UnitTextBox, calcMod1ComboBox)
    CheckTime(mapCd2FromTextBox, mapCd2ToTextBox, mapCd2UnitTextBox, calcMod2ComboBox)
    CheckTime(mapCd3FromTextBox, mapCd3ToTextBox, mapCd3UnitTextBox, calcMod3ComboBox)
    CheckTime(mapCd4FromTextBox, mapCd4ToTextBox, mapCd4UnitTextBox, calcMod4ComboBox)
    CheckTime(mapCd5FromTextBox, mapCd5ToTextBox, mapCd5UnitTextBox, calcMod5ComboBox)
    CheckTime(mapCd6FromTextBox, mapCd6ToTextBox, mapCd6UnitTextBox, calcMod6ComboBox)
    CheckTime(mapCd7FromTextBox, mapCd7ToTextBox, mapCd7UnitTextBox, calcMod7ComboBox)


    private void CheckTime(TextBox fromTb, TextBox ToTb, TextBox unitTb, ComboBox cb)
    {
       errorMessage = string.Empty;

       if (fromTb.Text.Trim().Length != 0)
       {
           if (toTbText.Trim().Length != 0)
           {
               if (int.Parse(fromTb.Text) < int.Parse(toTbText))
               {
                   if (unitTb.Text.Trim().Length != 0)
                   {
                       if (cb.Text.Trim().Length != 0)
                       {
                           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;
       }
    }

    実際、まだ改善すべきところはたくさんありますが、一度に説明すると混乱されるといけませんので、少しずつ行きましょう。

    2008年3月11日 15:37
    モデレータ
  • trapemiya様へ

    こんにちは、剣道です。


    昨日私が出張しましたので、返事が遅れまして、申し訳ございませんでした。
    今から、このサブルーチンの方法で、変数をまとめて、コードの修正を行いたいです。
    テスト終わりましたら、また報告させて頂きます。
    お忙しいところ、色々教えていただきまして、大変有難う御座いました。

    2008年3月13日 0:45