C#+Windowsフォームで作るダメな電卓例

概要

　入社して少しC#を学び作成した電卓のコードの指摘点を見直すことで自身の成長に繋げていこーというお話。
　作ったものはC#+Windows Formで作成した簡単な電卓です。
計算順序などはwindowsの電卓とおなじく左側の式から処理する方式となります。
ex) 1+6*3 = (1+6)*3 = 21

今回載せるコードは初回レビュー時のものとなります。指摘点を受けて直したバージョンも次の回で載せる予定です。

内容

calc.cs

using System;
using System.Drawing;
using System.Text;
using System.Windows.Forms;

namespace calc
{
    public partial class Form1 : Form
    {
        private bool init_flag = true; //  計算終了後の初期化フラグ

        public Form1()
        {
            InitializeComponent();
        }
        //  数値ボタン
        private void btnNumber_Click(object sender, EventArgs e)
        {
            textBoxCalc.Text += ((Button)sender).Text;
        }
　　　　 //  演算子ボタン
        private void Op_Click(object sender, EventArgs e)
        {
            textBoxCalc.Text += ((Button)sender).Text;
        }
        
        //  小数点ボタン
        private void point_Click(object sender, EventArgs e)
        {
            textBoxCalc.Text += '.';
        }
        
        //  クリア(C)ボタン
        private void Clear_Click(object sender, EventArgs e)  //  式の初期化
        {
            textBoxCalc.Text = String.Empty;
            init_flag = true;
        }
       
        private void eq_Click(object sender, EventArgs e)  //  式の計算を行う
        {
            int next_op_pos = 0
            int before_op_pos = 0;
            char ch = ' ';
            char next_op = ' ';
            char before_op = ' ';
            string num = string.Empty;
            double left=0.0; //演算子で式を分けた時の左と右の数値
            double right = 0.0;
            double result = 0.0;
            bool calc_flag = true;  //  計算したかどうかのフラグ
            bool op_flag = true;  //  取得した

            textBoxCalc.Text += '=';
             
            // フォームに何も入れずに"="を押した場合
            if(textBoxCalc.Text == "")
            {
                MessageBox.Show("式を入力してください。",
                    "入力エラー", MessageBoxButtons.OK, MessageBoxIcon.Error);
                return;
            }
            if (init_flag == false)//計算終了後に=を押したときの対策
                return;

            for (int i = 0; i < textBoxCalc.Text.Length;i++) //フォームから1文字ずつ読みだす
            {
                ch = textBoxCalc.Text[i];
                if (Char.IsDigit(ch) == true || ch == '.') //数値もしくは小数点の場合
                {
                    num += ch;    //数値のコピーをとっておく
                }
                else if (ch == '+' || ch == '-' || ch == '*' || ch == '/' || ch == '=')//いずれかの演算子の場合
                {
                    if (ch != '=')
                    {
                        if (op_flag == true) //  演算子の場合
                        {
                            before_op_pos = i;
                            before_op = ch;
                            op_flag = false;
                        }
                        else
                        {
                            next_op_pos = i;
                            next_op = ch;
                        }
     
                    }
                    
                    if (calc_flag)
                    {
                        left = double.Parse(textBoxCalc.Text.Substring(0, num.Length));//初回の計算のみleftを取得
                        calc_flag = false;
                        num = string.Empty; 
                    }
                    //  2回目以降の計算
                    else
                    {
                        right = double.Parse(textBoxCalc.Text.Substring(before_op_pos + 1, num.Length));
                        result = calc(left, right, before_op);
                        left = result;
                        num = string.Empty;
                        op_flag = true;
                        before_op_pos = next_op_pos;
                        before_op = next_op;

                    }
                }
                else
                    return;               
            }
            textBoxCalc.Text = result.ToString();
            init_flag = false;
            calc_flag = true;
           
        }
        //  計算用メソッド
        private double calc(double x, double y,char op)
        {
            double ans = 0.0;
            switch(op)
            {
                case '+':
                    ans =  x + y;
                    break;
                case '-':
                    ans = x - y;
                    break;
                case '*':
                    ans = x * y;
                    break;
                case '/':
                    ans = x / y;
                    break;
            }
            return ans;
        }        
    }
}

(Formデザイナー部分は省略)

改善点

・行き当たりばったりなロジック構成
→事前にデータフローなどの図を書いておらず、構成を把握しないままコーディングしている
・フォーム上の演算子や数値の位置を取得して計算をしているが、その必要は全くない。位置を取得するという考え自体が謎
・いらない変数が多い。特にフラグ
・変数名から変数の役割を推測できない
・計算結果を格納している変数resultは不要。leftだけで十分

終わりに

どうしてこんなになるまで(ry　な出来で恥ずかしいどころの騒ぎじゃないです(;_;)
逐次的に考えているため、いらない変数増やしたり、条件分岐増やしたりとダメダメすぎます...
改めてプログラミングの考え方がなってないと思いました....