Управление типами часть и графы


Обе функции выглядят довольно похожими друг на друга, но они различны в типа строка. У меня есть 6 функций и различаются лишь в "строке cmd и". Любой идеи, как я могу поместить все это в одно?

public void getFrmSingProgTbl(int flg)
    {
        if (flg == 1)
        {
            object[] astatus = new object[1];
            astatus = td.CheckOk(td.t1snd);
            string status = (string)astatus[0];
            if (status  == "true")
            {
                string cmd = getCurrTime() + "|" + "part-type" + "|" + t1prtNameTxt.Text + "\n";
                cmd += getCurrTime() + "|" + "operation-type" + "|" + t1oprNameTxt.Text + "\n";

                string showCmd = "Part Type - " + t1prtNameTxt.Text + "\nOperation - " + t1oprNameTxt.Text;
                if (MessageBox.Show(showCmd, "Confirm", MessageBoxButtons.YesNo) == DialogResult.Yes)
                {
                    HandleAfterSend(td.t1snd, td.t1stp, flg, td.t1btn);
                    sndData(cmd,1);
                }                   
            }
            else
            {
                this.ActiveControl = (TextBox)astatus[1];
                MessageBox.Show(status, "Error");
            }
        }
        else if (flg == 2)
        {
            object[] astatus = new object[1];
            astatus = td.CheckOk(td.t1stp);
            string status = (string)astatus[0];

            if (status == "true")
            {
                string cmd = getCurrTime() + "|" + "part-count-good" + "|" + t1gpTxt.Text + "\n";
                cmd += getCurrTime() + "|" + "part-count-bad" + "|" + t1bpTxt.Text + "\n";
                string showCmd = "Good Parts - " + t1gpTxt.Text + "\nBad Parts - " + t1bpTxt.Text;
                if (td.CheckNumeric(td.t1stp))
                {
                    if (MessageBox.Show(showCmd, "Confirm", MessageBoxButtons.YesNo) == DialogResult.Yes)
                    {
                        HandleAfterSend(td.t1stp, td.t1snd, flg, td.t1btn);
                        sndData(cmd,0);
                    }
                }
                else
                {
                    MessageBox.Show("Enter only numbers for Part Count","Error");
                }
            }
            else
            {
                this.ActiveControl = (TextBox)astatus[1];
                MessageBox.Show(status, "Error");
            }
        }
    }

public void getFrmSingFixtTbl(int flg)
    {
        if (flg == 1)
        {
            object[] astatus = new object[1];
            astatus = td.CheckOk(td.t3snd);
            string status = (string)astatus[0];
            if (status == "true")
            {
                string cmd = getCurrTime() + "|" + "part-type" + "|" + t3prtNameTxt.Text + "\n";
                cmd += getCurrTime() + "|" + "operation-type" + "|" + t3oprNameTxt.Text + "\n";
                cmd += getCurrTime() + "|" + "fixture-positions" + "|" + t3fixPosnTxt.Text + "\n";
                string showCmd = "Part Type" + " - " + t3prtNameTxt.Text + "\n";
                showCmd += "Operation" + " - " + t3oprNameTxt.Text + "\n";
                showCmd += "Parts per Fixture " + " - " + t3fixPosnTxt.Text + "\n";
                if (MessageBox.Show(showCmd, "Confirm", MessageBoxButtons.YesNo) == DialogResult.Yes)
                {
                    HandleAfterSend(td.t3snd, td.t3stp, flg, td.t3btn);
                    sndData(cmd,1);
                }
            }
            else
            {
                this.ActiveControl = (TextBox)astatus[1];
                MessageBox.Show(status, "Error");
            }
        }
        else if (flg == 2)
        {
            object[] astatus = new object[1];
            astatus = td.CheckOk(td.t3stp);
            string status = (string)astatus[0];
            if (status == "true")
            {
                string cmd = getCurrTime() + "|" + "part-count-good" + "|" + t3gpTxt.Text + "\n";
                cmd += getCurrTime() + "|" + "part-count-bad" + "|" + t3bpTxt.Text + "\n";
                string showCmd = "Good Parts" + " - " + t3gpTxt.Text + "\n";
                showCmd += "Bad Parts" + " - " + t3bpTxt.Text + "\n";
                if (td.CheckNumeric(td.t3stp))
                {
                    if (MessageBox.Show(showCmd, "Confirm", MessageBoxButtons.YesNo) == DialogResult.Yes)
                    {
                        HandleAfterSend(td.t3stp, td.t3snd, flg, td.t3btn);
                        sndData(cmd,0);
                    }
                }
                else
                {
                    MessageBox.Show("Enter only numbers for Part Count","Error");
                }
            }
            else
            {
                this.ActiveControl = (TextBox)astatus[1];
                MessageBox.Show(status, "Error");
            }
        }
    }


746
7
задан 29 апреля 2011 в 03:04 Источник Поделиться
Комментарии
4 ответа

Быстрые мысли с верхней части моей головы.


  1. Использовать стандартные .Конвенции чистая именования и не сокращайте

  2. Не менять всю обработку методом флагом

  3. Если вы контролируете тд.CheckOk(...) почему это возвратит массив, когда он не используется таким образом? Кроме того, она должна возвращать логическое значение, а не строку "True". Это должно возвратить определенного типа, а не кучу данных в различные индексы массивов.

  4. Почему вы создаете новый "статус" массив = новый объект[1]; и затем сразу же заменив его значением из тд.CheckOk?

  5. Использование класса StringBuilder, а не множество строк concats

  6. Похоже, вы могли бы заменить каждую функцию с (игнорируя все вышеперечисленные изменения):

    public void GetResult(Func<string> generateCommand, Func<string> generateSendData) {
    object[] astatus = td.CheckOk(td.t1snd);
    if (astatus != null && astatus.Length > 0 && (string)astatus[0] == "true") {
    string cmd = generateCommand();
    string data = generateSendData();
    if (MessageBox.Show(showCmd, "Confirm", MessageBoxButtons.YesNo) == DialogResult.Yes) {
    HandleAfterSend(td.t1snd, td.t1stp, flg, td.t1btn);
    sndData(cmd,1);
    }
    } else {
    this.ActiveControl = (TextBox)astatus[1];
    MessageBox.Show(astatus[0].ToString(), "Error");
    }
    }

И затем вызвать его:

GetResult(
() => getCurrTime() + "|" + "part-type" + "|" + t1prtNameTxt.Text + "\n" + getCurrTime() + "|" + "operation-type" + "|" + t1oprNameTxt.Text + "\n",
() => "Part Type - " + t1prtNameTxt.Text + "\nOperation - " + t1oprNameTxt.Text
);
GetResult(
() => getCurrTime() + "|" + "part-count-good" + "|" + t1gpTxt.Text + "\n" + getCurrTime() + "|" + "part-count-bad" + "|" + t1bpTxt.Text + "\n",
() => "Good Parts - " + t1gpTxt.Text + "\nBad Parts - " + t1bpTxt.Text
);

//etc

15
ответ дан 29 апреля 2011 в 04:04 Источник Поделиться


  1. Не используйте инт для флага. Использование типа bool или перечисление

  2. Не используйте строку для статуса. Использование типа bool

  3. Не использовать объект[] за "статус". Написать собственный класс для этого

  4. Переписать все держать первые 3 очка в уме

14
ответ дан 29 апреля 2011 в 04:04 Источник Поделиться

Чтобы добавить в akmad ответ можно упростить обработки строк с использованием строки маску и строки.Формат() функция.

Например:

string cmd = getCurrTime() + "|" + "part-count-good" + "|" + t1gpTxt.Text + "\n";
cmd += getCurrTime() + "|" + "part-count-bad" + "|" + t1bpTxt.Text + "\n";
string showCmd = "Good Parts - " + t1gpTxt.Text + "\nBad Parts - " + t1bpTxt.Text;

могут быть заменены:

string cmdFormat = "{0}|part-count-good|{1}\n{0}|part-count-bad|{2}\n";
string cmd = String.format(cmdFormat, getCurrTime(), tlgp.Text, tlbp.Text);

string showFormat = "Good Parts - {0}\nBad Parts - {1}";
string showCmd = String.Format(showFormat, tlgpTxt.Text, tlbpTxt.Text);

5
ответ дан 29 апреля 2011 в 09:04 Источник Поделиться

Имена методов, которые начинаются с "вам" просто звучит неправильно для методов, которые возвращают Void.

В зависимости от того, как используются эти методы, вы сможете извлекать их для командных объектов с общей части, реализуемой в общий базовый класс и различия экстрагируют для абстрактных методов.

5
ответ дан 30 апреля 2011 в 12:04 Источник Поделиться