Калькулятор CC C #

Это мой основной калькулятор C #; Я уверен, что многое можно улучшить. Этот дизайн основан на калькуляторе C ++ Bjarne Stroustrup, специально преднамеренной версии которого можно найти на своем веб-сайте .

enum Operators
{
    Addition, Subtraction, Multiplication, Division, Modulo
}

static class ExtensionMethods
{
    public static string RemoveAll(this string str, char toRemove)
    {
        for (int i = 0; i < str.Length; i++)
        {
            if (str[i] == toRemove)
            {
                str = str.Remove(i, 1);
                i--;
            }
        }
        return str;
    }
}

class Calculator
{
    private static StreamReader dataStream;

    private static Operators getNextOperator()
    {
        const string OperatorSet = "+-*/%";

        char token = (char)dataStream.Read();

        if (OperatorSet.IndexOf(token) == -1)
        {
            throw new InvalidDataException("Invalid or missing operator.");
        }
        return (Operators)OperatorSet.IndexOf(token);
    }

    private static double getNextNum()
    {
        string value = string.Empty;

        while (true)
        {
            if ("0123456789.".IndexOf((char)dataStream.Peek()) == -1)
            {
                break;
            }

            value += (char)dataStream.Read();
        }

        double d;
        if (double.TryParse(value, out d))
        {
            return d;
        }
        Console.WriteLine(value);
        throw new InvalidDataException("Expected a number.");
    }

    private static double NumbersAndParens()
    {
        bool valueIsNegative = false;
        double val = -1;    // trick the compiler

        if ((char)dataStream.Peek() == '-')
        {
            valueIsNegative = true;
            dataStream.Read();
        }

        if ((char)dataStream.Peek() == '(')
        {
            dataStream.Read();
            val = AdditiveOperators();

            if ((char)dataStream.Peek() == ')')
            {
                dataStream.Read();
                return valueIsNegative ? -1 * val : val;
            }
            else
            {
                throw new InvalidDataException("Expected closing parenthesis.");
            }
        }

        val = getNextNum();

        return valueIsNegative ? -1 * val : val;
    }

    private static double MultiplicativeOperators()
    {
        double val = NumbersAndParens();

        while (true)
        {
            if ("*/%".IndexOf((char)dataStream.Peek()) == -1)
            {
                return val;
            }

            Operators op = getNextOperator();

            if (op == Operators.Multiplication)
            {
                val *= NumbersAndParens();
            }
            else if (op == Operators.Division)
            {
                double nextNum = NumbersAndParens();
                if (nextNum == 0)
                {
                    throw new DivideByZeroException("Divide by 0 error.");
                }
                val /= nextNum;
            }
            else
            {
                val %= NumbersAndParens();
            }
        }
    }

    private static double AdditiveOperators()
    {
        double val = MultiplicativeOperators();

        while (true)
        {
            if ("-+".IndexOf((char)dataStream.Peek()) == -1)
            {
                return val;
            }
            Operators op = getNextOperator();

            if (op == Operators.Addition)
            {
                val += MultiplicativeOperators();
            }
            else if (op == Operators.Subtraction)
            {
                val -= MultiplicativeOperators();
            }
        }
    }

    private static Stream StringToStream(string equation)
    {
        MemoryStream stream = new MemoryStream();
        StreamWriter writer = new StreamWriter(stream);

        writer.Write(equation.RemoveAll(' '));
        writer.Flush();

        stream.Position = 0;
        return stream;
    }

    /// <summary>
    /// 
    /// </summary>
    /// <param name="equation">A mathematical epxression needing evalation.</param>
    /// <returns>The evaluated expression.</returns>
    public static double EvaluateExpression(string equation)
    {
        dataStream = new StreamReader(StringToStream(equation));

        double evaluation = AdditiveOperators();

        if (!dataStream.EndOfStream)
        {
            throw new InvalidDataException("Invalid expression.");
        }

        return evaluation;
    }
}
11 голосов | спросил Hosch250 25 AMpSat, 25 Apr 2015 07:45:16 +030045Saturday 2015, 07:45:16

3 ответа


11

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


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

В коде:

var input = "19*cqwcq521sq0dqs0-dqs44sq";
var toRemove = 'c';
var result = new string(input.ToCharArray().Where(x => x != toRemove).ToArray());

Почему все статично? Ваши методы используют общий ресурс (StreamReader), который должен указывать, что на самом деле это что-то , но static


Соглашения об именах C # диктуют метод UpperCamelCase для методов.


if (OperatorSet.IndexOf(token) == -1)
{
    throw new InvalidDataException("Invalid or missing operator.");
}
return (Operators)OperatorSet.IndexOf(token);

Мне не нравится двойная работа. Сохраните результат IndexOf в промежуточной переменной.


getNextNum()

Num не является приемлемой комбинацией в Scrabble, таким образом, мы не разрешаем это и здесь. Напишите его полностью.


 if (OperatorSet.IndexOf(token) == -1)

, также известный как

 if(!"+-*/%".Contains(token))

Если вы собираетесь использовать это подробно, рассмотрите методы расширения на char: IsOperator(), IsDigit() и т. Д.

public static bool IsDigit(this char c)
{
    return "123456789".Contains(c);
}

while (true)
{
    if ("0123456789.".IndexOf((char)dataStream.Peek()) == -1)
    {
        break;
    }

    value += (char)dataStream.Read();
}

Пришло время для веселого случая кромки!

Вопрос: что происходит, когда у вас есть большое количество reaaaaaaaa? Я говорю reaaaaaaallllllyyyyy большой. Я говорю 308 символов.

Правильно, это будет правильное значение, но ваш код выведет код InvalidDataException, когда он не сможет разобрать его как double. Аналогично: если в вашей строке есть символы int.MaxValue, у вас также будут проблемы. Хотя в этот момент ваша память будет уже исчерпана, поэтому она «хорошо».

Если вы не строите что-то, предназначенное для этих случаев: не исправляйте это. Я просто подумал, что стоит упомянуть.


Вы , используя конкатенацию строк вместо StringBuilder. Не используйте конкатенацию в цикле.


Console.WriteLine(value);
throw new InvalidDataException("Expected a number.");

Не делайте никаких (бесполезных?) выходов из вашего калькулятора. Это работа для любых классов, которые потребляют ваш калькулятор.


valueIsNegative

должен быть

isNegative

double val = -1;    // trick the compiler

Вы также обманываете красивого рецензента. Почему вы это делаете?


if ((char)dataStream.Peek() == '-')
{
    valueIsNegative = true;
    dataStream.Read();
}

Что делать, если у меня есть --9 как вход? Это позитивно, но вы скажете мне, что это отрицательно.


Почему вы используете .Peek(), чтобы получить значение, а затем поместив бесполезный вызов на .Read(), чтобы отменить значение? Это не значит, что поток предназначен для использования: просто вызовите .Read() с самого начала.


if ((char)dataStream.Peek() == '(')
{
    dataStream.Read();
    val = AdditiveOperators();

Почему вы предполагаете, что после открытой круглой скобки у меня будет «аддитивный» оператор?


MultiplicativeOperators()

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


NumbersAndParens()

Способы описывают действие. Это не описывает действие.


else
{
    val %= NumbersAndParens();
}

У вас есть провал в коде, который очень слабо связан: небольшая ошибка в одном из нескольких мест, и вдруг вам очень сложно отслеживать ошибку. Используйте явную ветвь else if(op == Operator.Modulo) и добавьте исключение.


Я не вижу никакой разницы междуMultiplicativeOperators() и AdditiveOperators(), который гарантирует, что они находятся в различные методы.


MemoryStream stream = new MemoryStream();
StreamWriter writer = new StreamWriter(stream);

using blocks!

ответил Jeroen Vannevel 25 PMpSat, 25 Apr 2015 15:27:29 +030027Saturday 2015, 15:27:29
10

Использовать особые имена для перечислений

enum Operators
{
    Addition, Subtraction, Multiplication, Division, Modulo
}

Псевдоним для перечислений на практике немного неестественен. Посмотрите на это:

private static Operators getNextOperator() { ... }

Operators op = getNextOperator();

Похоже, что тип - это какая-то коллекция, например, набор или список или карта. Множественное «Операторы» и единственное «getNextOperator ()» также запутывает.

Итак, переименуйте его в сингулярный Operator, и все его использование становится естественным.

Я также удивлен, что операторы не определены вместе с их обычными символами, я вижу их только имена.

Централизовать символы оператора

Символы оператора разбросаны по коду. Различные методы Calculator используют магические строки типа "+-*/%", "-+", "*/%". У вас уже есть перечисление для операторов, было бы лучше, чтобы он отвечал за символы.

Хуже всего такого рода вещи:

private static Operators getNextOperator()
{
    const string OperatorSet = "+-*/%";

Почему OperatorSet локальная переменная? Это должно было быть как минимум статической константой.

Что-то вроде этого было бы лучшим подходом:

private static Operator getNextOperator()
{
    return Operator.fromChar((char)dataStream.Read());
}

, где реализация Operator.fromChar должна генерировать исключение, если символ недействителен.

Реализация классов утилиты

Класс ExtensionMethods - это класс утилиты. Его название предполагает гораздо больше, чем то, что оно есть. По крайней мере, на данный момент он содержит единственный метод, который манипулирует строкой, поэтому StringUtils будет более подходящим именем.

A switch является более естественным для ветвления на перечислениях

Перечисления хорошо подходят для операторов switch вместо цепочки if-else. Это не проблема, а просто более общий стиль письма.

getNextNum является полупеченным

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

Реализация выглядит полуобжаренной, потому что вы читаете характер по характеру, выполняя некоторые, но не все необходимые проверки с помощью dataStream.Peek(). Эффективно, вы делаете половину разбора, и оставляя остальных double.TryParse, который также неизбежно повторит уже выполненные проверки цифр.

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

Именование булей

Это может быть вопросом вкуса, но вместо этого:

    bool valueIsNegative = false;

Я предлагаю следующее:

    bool negative = false;

В соответствующей заметке это может быть немного проще:

valueIsNegative ? -1 * val : val;

вот так:

valueIsNegative ? -val : val;

Удаление символов из строк

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

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

Короче говоря, реализация ExtensionMethods.RemoveAll выглядит неэффективной, и я предлагаю подумать о чем-то лучшем.

ответил janos 25 AMpSat, 25 Apr 2015 08:31:35 +030031Saturday 2015, 08:31:35
5

Есть 2 хороших ответа от @janos и @Jeroen. Я предлагаю несколько кратких дополнений в дополнение к их длинным ответам.

Метод RemoveAll

Я предлагаю больше полагаться на структуру.

public static string RemoveAll(this string str, char toRemove)
{
    return str.Replace(toRemove.ToString(), string.Empty);
}

valueIsNegative и -9

Что касается valueIsNegative и вопрос Jeroen о том, как обращаться с --9, я предлагаю переворачивать знак каждый раз, когда вы сталкиваетесь с ним.

if ((char)dataStream.Peek() == '-')
{
    valueIsNegative = !valueIsNegative;
    dataStream.Read();
}
ответил Rick Davin 25 PMpSat, 25 Apr 2015 16:55:01 +030055Saturday 2015, 16:55:01

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132