Калькулятор-как приложение


Я пытаюсь реализовать калькулятор-приложение на C#. Но то, как я в настоящее время реализован, это кажется немного сумбурно и неэффективно. Это работает, но я просто уверен, что он может быть достигнут в более элегантное решение.

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

Вот основная часть кода:

private double Calculate()
{

    double answer = 0.0;
    foreach (double number in this.numbers)
    {
        int index = this.numbers.IndexOf(number);

        if (index != this.numbers.Count() - 1)
        {
            if (answer != 0.0)
            {
                switch ((int)operators[index])
                {
                    case (int)Operator.Plus:
                        answer = (double)(answer + numbers[index + 1]);
                        break;

                    case (int)Operator.Minus:
                        answer = (double)answer - numbers[index + 1];
                        break;

                    case (int)Operator.Divide:
                        answer = (double)answer / numbers[index + 1];
                        break;

                    case (int)Operator.Multiply:
                        answer = (double)answer * numbers[index + 1];
                        break;
                }
            }
            else
            {
                switch ((int)operators[index])
                {
                    case (int)Operator.Plus:
                        answer += (double)(number + numbers[index + 1]);
                        break;

                    case (int)Operator.Minus:
                        answer += (double)number - numbers[index + 1];
                        break;

                    case (int)Operator.Divide:
                        answer += (double)number / numbers[index + 1];
                        break;

                    case (int)Operator.Multiply:
                        answer += (double)number * numbers[index + 1];
                        break;
                }
            }
        }       

    }

    return answer;
}

Просто для справки, операторы-перечислимый, который определен как:

private enum Operator
{
    Plus = 1,
    Minus = 2,
    Multiply = 3,
    Divide = 4,
}

и два списка:

private List<double> numbers;
private List<Operator> operators;

Любой идеи, как я могу улучшить это?



1266
7
задан 26 октября 2011 в 10:10 Источник Поделиться
Комментарии
5 ответов

Ваш первый большой проблемой будет бороться ваш дизайн. Несколько случае подобные заявления просят быть удалены либо из-за полиморфизма, или глубоко в миски какую-то AbstractFactory, и только один раз (к сожалению, я только что на чистый код книги).
В этом случае, что делает эта установка полиморфных собирается быть твоим другом... в противном случае, вы будете иметь интересное время спустя, пытаюсь добавить, скажем modulous (%).

Для начала, определите работу интерфейса. Что-то вроде этого:

public interface Operation {
public double Operate(double argA, double argB);
}

Тогда нужно отдельно выполнять каждую операцию:

public struct Division : Operation {
// For obvious reasons, the proper argument names should be specified
public double Operate(double quotient, double divisor) {
return quotient / divisor;
}
}

Это позволит вам добавить любое количество будущих операций довольно просто (в том числе некоторые действительно сложные вещи) - так долго, как он только принимает 2 аргументов... (заметьте - я пытаюсь сохранить общую концепцию вашего дизайна; есть и другие возможности здесь). И заменить список с Список.

Тогда ваш способ расчета становится, просто:

private double Calculate () {
double answer = 0.0;
for(int i = 0; i < numbers.Count(), i < operators.Count(), i++) {
// Of course, this doesn't check for divide-by-zero...
answer = operators[i].Operate(answer, numbers[i]);
}
return answer;
}

Ваш текущий метод имеет следующие проблемы -


  1. При перечислении нескольких экземплярах с тем же номером, но с разными операциями, хотел бы повторно использовать только одну из операций (вероятно, первый в списке).

  2. Вы не достаточно проверить для массива вне границ ошибки - если цифры больше, чем операторов, вы будете запускать из операторов (это спорно, если игнорировать цифры на конце лучше, как я делаю...).

  3. Ваш если (ответ == 0.0) заявление действительно отражает точное же логике. Вы просто должны смотреть на это немного иначе.

7
ответ дан 26 октября 2011 в 11:10 Источник Поделиться

Ну, во-первых, есть ошибка в коде. Вы получаете индекс номер, ищу его в списке, но если количество произошло в начале списка вы получите неправильный индекс. Вы должны использовать индекс в цикле вместо использования оператора foreach.

Используется функция count() метод расширения, чтобы получить количество элементов в списке, но вы должны использовать граф собственность вместо.

Вы бросаете все значения enum к int, но вы можете просто использовать enum значения напрямую.

Вы бросаете много значений в два раза, но они уже двойные значения.

Вы используете значение ответа в коде, где вы определили, что это всегда ноль.

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

Вместо обработки первой итерации по-разному, можно просто поставить первым номером в ответ переменную и запустить цикл от 1 года.

Код вдруг все стало намного проще. :)

private double Calculate() {
double answer = numbers[0];
for (int index = 1; index < numbers.Count; index++) {
switch (operators[index - 1]) {
case Operator.Plus:
answer += numbers[index];
break;
case Operator.Minus:
answer -= numbers[index];
break;
case Operator.Divide:
answer /= numbers[index];
break;
case Operator.Multiply:
answer *= numbers[index];
break;
}
}
return answer;
}

12
ответ дан 26 октября 2011 в 11:10 Источник Поделиться

Х-ноль имеет некоторые хорошие предложения. В зависимости от предполагаемой сложности вашей программы, создание полиморфных конструкция как X-ноль предположил, безусловно, хороший способ пойти, и это делает будущее код гораздо проще. Я бы просто добавить несколько замечаний по этим предложениям код, хотя:


  • Особенно если этот код может быть использован в качестве той или иной форме публичного API, это может быть хорошей идеей, чтобы назвать правильно интерфейса и реализации. Например, операции - это интерфейс, так что следующие стандартные соглашения об именовании должно быть изменено на IOperation. Аналогично, может быть, стоит назвать реализаций как SomethingOperation. Например, дивизия станет DivisionOperation. Это не 100% необходимо, но делает его легче определить, какой класс является реализацией, не глядя в код.

  • Еще один момент я хотел бы сделать, заключается в том, что в течение цикла, Первая операция выполняется с использованием параметров по умолчанию начальное значение ноль. Если первой операцией является умножение, этот метод приведет к ошибочным результатам. Чтобы решить эту проблему, я хотел бы сослаться на ответ, данный Guffa:


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

    Вместо обработки первой итерации по-разному, можно просто поставить первое число в переменной ответа и запустить цикл от 1 года.

Примечание: точку зрения на начальное значение ноль имел в виду ответ х-ноль, а не код в вопрос.

1
ответ дан 27 октября 2011 в 07:10 Источник Поделиться

Как люди думают о замене перечисления классов и перенос логики в нам библиотеки и функции?

public class Operation<T>
{
public string Name { get; set; }
public Func<T, T, T> Calculation { get; set; }
}

private List<double> numbers;
private List<Operation<double>> operations = new List<Operation<double>>
{
new Operation<double> {Name = "Add", Calculation = (x,y) => x + y},
new Operation<double> {Name = "Subtract", Calculation = (x,y) => x - y},
new Operation<double> {Name = "Multiply", Calculation = (x,y) => x * y},
new Operation<double> {Name = "Divide", Calculation = (x,y) => x / y}

//,... Add new ones as you like here
};

public double Calculate<T>()
{
var answers = numbers[0];
for (int index = 0; index < operations.Count; index++)
{
var operation = operations[index];
operation.Calculation(answers, numbers[index]);
}

return answers;
}

1
ответ дан 4 ноября 2011 в 08:11 Источник Поделиться

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

bool answer_zero = (bool)(answer != zero);
switch ((int)operators[index])
{
case (int)Operator.Plus:
answer = (double)((answer_zero ? answer : number) + numbers[index + 1]);
break;

case (int)Operator.Minus:
answer = (double)((answer_zero ? answer : number) - numbers[index + 1]);
break;

case (int)Operator.Divide:
answer = (double)((answer_zero ? answer : number) / numbers[index + 1]);
break;

case (int)Operator.Multiply:
answer = (double)((answer_zero ? answer : number) * numbers[index + 1]);
break;
}

0
ответ дан 26 октября 2011 в 11:10 Источник Поделиться