Версия для печати темы Нажмите сюда для просмотра этой темы в оригинальном формате

Форум программистов > C++ Builder > Калькулятор с одним полем TEdit.

Автор: Dilettante 15.10.2012, 18:12

Приветствую вас! Имеется калькулятор, состоящий из следующих частей:  поле Edit, в которое вводятся опреанды и выдаётся результат  кнопки операций   кнопка "равно"  кнопка очистки Операции и операнды вносятся в очередь, а после нажатия кнопки "равно" происходит обработка накопленного выражения. Вобщем, код, реализующий вычисления, следующий: Код void __fastcall TForm1::Operation(TObject *Sender) {     enum { sum = 1, sub, mul, div, intmod, intdiv, clear, equal = 9 };     int                        tag   = ((TButton*)Sender)->Tag;     bool                       error = false, prnths = false;     try {         op = StrToFloat(OpResEdit->Text);     }     catch(EConvertError&)     {         if(!OpResEdit->Text.IsEmpty())         {             OpResEdit->Text = "error: incorrect input";             return;         }     }     if(!OpResEdit->Text.IsEmpty())         old = op;     else         op = old;     operands.push(op);     if(tag == equal)     {         try {             res = operands.front();             operands.pop();             while(!operators.empty())             {                 operation = operators.front();                 op        = operands.front();                 if((operation == '/' || operation == 'm' || operation == 'd') && !op)                     throw EZeroDivide("error: division by zero");                 switch(operation)                 {                     case '+': res += op;                break;                     case '-': res -= op;                break;                     case '*': res *= op;                break;                     case '/': res /= op;                break;                     case 'm': res = (int)res % (int)op; break;                     case 'd': res = (int)res / (int)op; break;                 }                 operators.pop();                 operands.pop();             }         }         catch(EZeroDivide& exc)         {            OpResEdit->Text = exc.Message;            error = true;         }         if(!error)             OpResEdit->Text = FloatToStr(res);         OpLbl->Caption  = "";         res = 0;     }     else     {         if(!operators.empty())             prnths = ((operators.back() == '*' || operators.back() == '/') ? false : true) ;         switch(tag)         {             case sum:     operators.push('+'); break;             case sub:      operators.push('-'); break;             case mul:      operators.push('*'); break;             case div:       operators.push('/'); break;             case intmod: operators.push('m'); break;             case intdiv:   operators.push('d'); break;         }         if((operators.back() == '*' || operators.back() == '/') && prnths)             OpLbl->Caption = "(" + OpLbl->Caption + FloatToStr(op) + ")" + operators.back();         else             OpLbl->Caption = OpLbl->Caption + FloatToStr(op) + operators.back();         OpResEdit->Text = FloatToStr(op);     }     cls = true; } Переменные в области private: Код     bool                             cls;     double                         op, old;     double                         res;     wchar_t                       operation;     std::queue<double>   operands;     std::queue<wchar_t> operators; Код для кнопки очистки: Код void __fastcall TForm1::ClearBtnClick(TObject *Sender) {     OpResEdit->Text = "";     OpLbl->Caption  = "";     res = op = old = 0;     while(!operators.empty())     {         operators.pop();         operands.pop();     }     operands.pop(); } Подскажите пожалуйста, всё ли здесь корректно в части вычислений? Попробовал посчитать разные выражения - вроде нормально. Но не уверен, что всё предусмотрено.

Автор: borisbn 16.10.2012, 08:56

Цитата(Dilettante @  15.10.2012,  18:12 ) Подскажите пожалуйста, всё ли здесь корректно в части вычислений? В части вычислений вроде всё корректно. Если позволишь, придерусь к стилю. 1) Для обработки кнопки "равно" лучше сделать отдельный обработчик, т.к. часть, отвечающая за обработку "равно" никак не связана с частью, отвечающей за обработку операций 2) В чём смысл держать double res; и wchar_t operation; членами класса ? Почему не объявить их прямо в функции-обработчике ? 3) Зачем нужно преобразование из enum'а в символ и обратно, если можно хранить очередь из enum'ом ? 4) (Необязательное) Чтобы отличать члены класса от переменных, объявленных в функции, у первых д.б. какой-нибудь признак. Иногда используют подчёркивание в конце имени, иногда префикс m_ - на твой вкус.

Автор: artsb 16.10.2012, 12:06

Цитата(borisbn @  16.10.2012,  08:56 ) 4) (Необязательное) Чтобы отличать члены класса от переменных, объявленных в функции, у первых д.б. какой-нибудь признак. Иногда используют подчёркивание в конце имени, иногда префикс m_ - на твой вкус.  Можно просто писать this->MyVar. Так точно не запутаешься )

Автор: Dem_max 16.10.2012, 12:15

Я так понял если вводить такое выражение 1+6*7-3/8 = ???? какое число выведет твой калькулятор ?

Автор: Dilettante 16.10.2012, 16:44

2borisbn, artsb Спасибо за замечания. Об отдельном обработчике для равенства я не подумал. Как-то включил его машинально к операциям. Вообще у меня изначально обработка всего поведения, связанного с полем TEdit, скопилась в одном методе.   Насчёт очереди из символов, с одной стороны, я к ней ещё обращаюсь при выводе строки, хранящей последовательность операций:  Код if((operators.back() == '*' || operators.back() == '/') && prnths)     OpLbl->Caption = "(" + OpLbl->Caption + FloatToStr(op) + ")" + operators.back(); else     OpLbl->Caption = OpLbl->Caption + FloatToStr(op) + operators.back(); OpResEdit->Text = FloatToStr(op); Ну а с другой - опять же, не подумал. Совет про именование переменных класса учту. 2Dem_max Выведет 5,75, т.к. не соблюдает приоритеты операций и считает такое: ((1+6)*7-3)/8.  2All Спасибо вам за внимание к моей проблеме! Попытаюсь сдать (лабораторная, как вы, возможно, догадались).

Автор: borisbn 16.10.2012, 16:59

Цитата(Dilettante @  16.10.2012,  16:44 ) Насчёт очереди из символов, с одной стороны, я к ней ещё обращаюсь при выводе строки, хранящей последовательность операций:  на здоповье. сравнивай так Код if((operators.back() == mul || operators.back() == div) && prnths) а для их отображения сделай функцию перевода enum'а в символ и вызывай её Цитата OpLbl->Caption = OpLbl->Caption + FloatToStr(op) + op_2_char( operators.back() ); сама функция op_2_char, как ты понимаешь, будет совсем несложная ))

Автор: Dilettante 16.10.2012, 17:12

2borisbn Да, удачно получается   Стоит так и сделать. Спасибо!

Автор: Dem_max 16.10.2012, 17:32

Цитата Выведет 5,75, т.к. не соблюдает приоритеты операций и считает такое: ((1+6)*7-3)/8.  Ну что тогда покритикую как просил - реализация фиговая.

Автор: Dilettante 16.10.2012, 17:57

2Dem_max Тут вы правы. На большее не хватило. Но я надеюсь, что для моих нужд будет достаточно.

Автор: artsb 16.10.2012, 20:02

Цитата(Dilettante @  16.10.2012,  16:44 ) Насчёт очереди из символов Тут всё намного проще: Код enum  {      sum = 1,      sub,      mul,      div,      intmod,      intdiv,      clear,      equal = 9  }; const char oper_str[] =  {     0,        // 0     '+',    // 1     '-',    // 2     '*',    // 3     '/',    // 4     'm',    // 5     'd',    // 6     0,        // 7     0,        // 8     0        // 9 }; И теперь можно так: Код OpLbl->Caption = OpLbl->Caption + FloatToStr(op) + oper_str[operators.back()]; Это будет работать быстрее, чем вызов функции. Надеюсь, идея понятна.

Автор: borisbn 16.10.2012, 21:41

>    Это будет работать быстрее, чем вызов функции. Готов поспорить, что дааааааааапалеко не всегда. Сферических коней в вакууме не предлагать

Автор: artsb 16.10.2012, 23:09

Цитата(borisbn @  16.10.2012,  21:41 ) Готов поспорить, что дааааааааапалеко не всегда. Ну, смысла спорить я не вижу, т.к. тут всё очевидно, ИМХО. При обращении к элементу массива ничего особенного не происходит. А вот при вызове функции, происходт много чего. Например, передача параметра; переключение на вызываемую функцию; работа в самой функции (скорее всего, там будет switch); возвращение значения из функции, что влечёт за собой создание временного объекта. Возможно, ещё что-то упустил. Как бы там ни было, на всё это нужно время. Поправьте меня, если я не прав.

Автор: borisbn 17.10.2012, 06:24

> Поправьте меня, если я не прав. Теоретически (как в том анекдоте) Вы правы, а практически компилятор сделает из switch'а таблицу, и заинлайнит функцию. Проверьте. Сделайте два варианта и посмотрите ассемблерный код (с включенной оптимизацией, ессно)

Автор: artsb 17.10.2012, 07:51

Цитата(borisbn @  17.10.2012,  06:24 ) (с включенной оптимизацией, ессно)  Вот. Ключевой момент ) И тем не менее, даже если функция будет оптимизирована и заинлайнится, неужели простая операция с адресом (указатель на массив + смещение) будет длить дольше? Добавлено через 2 минуты и 17 секунд Кстати, обе операции (мой вариант и ваш вариант с функцией) будут отрабатывать очень быстро. Настолько, что разницу заметить очень тяжело.

Автор: Dilettante 17.10.2012, 16:18

Использование массива мне нравится (и кажется наиболее быстрым, т.к., наверное, O(1) не идёт из головы   ), хотя работу уже сдал с правками в стиле borisbn).  borisbn, artsb, Dem_max,  Спасибо вам за помощь и внимание к теме!

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)