Калькулятор на деревьях, C#. С удовольствием выслушаю почему я не прав (о качестве кода)?

Question

[nsinreal]

Захотелось мне написать калькулятор: paste.org.ru/?nnq9gg

Что удивительно — работает, проходит многие мои тесты. Но мне не чем-то нравится код. Нету красоты.

Что делать?

Comments

[catlion]

Что для вас есть красота?

[nsinreal]

К примеру, ф-я Build длинная и разветвленная и одним взглядом её не оценить как она работает. Но я не могу придумать как сделать её проще и читабельнее.

[nsinreal]

Да, и еще один момент, который меня смущает — однотипные действия все равно расписываются через бинарное дерево, а как сделать так, чтобы они были в одной ноде, но при этом код не стал сложнее (лишними проверками) — я не понимаю

Answer 1

[tick]

медитировать

Comments

[nsinreal]

а поконкретнее?

[catlion]

Перепишите на F#, ФВП и рекурсия добавят красоты :) А если совсем серьезно, у каждого свое понятие о красоте. Мне код понравился, чистенько и доходчиво.

[catlion]

А еще можно пройтись по коду StyleCop и посмотреть, как это будет выглядеть в соответствии с рекомендациями по оформлению от Microsoft.

Answer 2

[Алексей Кулаков]

Сугубо мои предпочтения.

Не понятно с первого взгляда какая переменная какой уровень имеет (глобальная публичная, глобальная зактытая, локальная, параметр метода). Есть на этот счет некоторые правила именования, например, глобальные для класса
private string _nameOfVariable,
public string NameOfVariable,
локальные просто с маленькой буквы как и параметры методов.

Стилистика имен везде разная - то m_Value, а то lexemType.

Имена методов на мой вкус должны все же отражать какое действие они совершают. Например, есть метод LexemType() Увидишь такой и нужно лезть в код, чтобы понять чего он делает. Лучше было бы, например GetLexemType() или GetLexemByChar(), но в основном вы придерживаетесь этой схемы.

Имена классов уж очень сокращенные. По мне имена Expression, ExpressionVariable, ExpressionTree и прочее информативнее. Не нужно угадывать, что там автор имел в виду и какое сокращение придумал.

Зачем присваивать 0 переменной double как 0.0? Там и так уже указан тип.

Лишние вызовы методов. Зачем делать Name.ToString() если Name уже string ?

Напрямую открывать поля класса тоже не хорошо. Можно сделасть свойство ну то есть вот так
public string Name {get; set;} или написать пару методов GetName() и SetName()

Ну это то, что бросилось сразу в глаза. По самой реализации может позже отпишусь, если время будет.

Answer 3

[Kir ---]

Уменьшать цикломатическую сложность. Она слишком высокая.