Польский переводчик счисления/калькулятор


Просто за фигню я написал простой переводчик, который принимает вклады в виде польской записи.

Пользователь будет вводить что-то вроде

+ 5 6

И это будет выход

11

Вот еще один пример ввода:

+ 5 * 2 3

А на выходе:

11

Пользователи могут даже определить свои собственные переменные, как это:

деф аргумент myVar 10

и такой:

деф anotherVar + 5 6

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

аргумент myVar

это будет выход

10

или

anotherVar

какие выходы

11

Кроме того, вы тип в

+ аргумент myVar anotherVar

тогда вы получили бы

21


Я полный новичок в хорошо отформатированный и легко читаемый код. Что бы быть хорошим способом, чтобы переработать этот?

#include <iostream>
#include <vector>
#include <string>
#include <sstream>

using namespace std;

class Variable
{
public:
    Variable(const string& name, double val)
    {
        this->name = name;
        this->val = val;
    }

    inline const string& get_name() const { return name; }
    inline double get_val() const { return val; }
private:
    string name;
    double val;
};

//----------------------------------

double operate(const string& op, istringstream& iss, vector<Variable>& v);
double perform_addition(istringstream& iss, vector<Variable>& v);
double perform_subtraction(istringstream& iss, vector<Variable>& v);
double perform_division(istringstream& iss, vector<Variable>& v);
double perform_multiplication(istringstream& iss, vector<Variable>& v);
void define_new_var(vector<Variable>& v, istringstream& iss);

bool is_number(const string& op)
{
    int char_to_int = op[0];
    if (char_to_int >= 48 && char_to_int <= 57)
        return true;

    return false;
}

void print_var_list(vector<Variable>& v)
{
    int size = v.size();
    for (int i = 0; i < size; i++)
    {
        cout << v[i].get_name() << ", " << v[i].get_val() << endl;
    }
    cout << endl;
}

int main()
{
    cout << endl << "LeRPN Programming Language" << endl;

    vector<Variable> v;

    string temp;
    while (cin)
    {
        cout << endl << "> ";

        getline(cin, temp);

        istringstream iss(temp);
        string op;
        iss >> op;

        if (op == "quit")
            break;
        else if (op == "def")
            define_new_var(v, iss);
        else if (op == "show_vars")
            print_var_list(v);
        else
            cout << endl << operate(op, iss, v) << endl;
    }
}

double perform_addition(istringstream& iss, vector<Variable>& v)
{
    string left;
    iss >> left;

    string right;
    iss >> right;

    return operate(left, iss, v) + operate(right, iss, v);
}

double perform_subtraction(istringstream& iss, vector<Variable>& v)
{
    string left;
    iss >> left;

    string right;
    iss >> right;

    return operate(left, iss, v) - operate(right, iss, v);
}

double perform_division(istringstream& iss, vector<Variable>& v)
{
    string left;
    iss >> left;

    string right;
    iss >> right;

    return operate(left, iss, v) / operate(right, iss, v);
}

double perform_multiplication(istringstream& iss, vector<Variable>& v)
{
    string left;
    iss >> left;

    string right;
    iss >> right;

    return operate(left, iss, v) * operate(right, iss, v);
}

double get_variable(const string& op, vector<Variable>& v)
{
    int size = v.size();
    for (int i = 0; i < size; i++)
    {   
        if (op == v[i].get_name())
            return v[i].get_val();
    }
}

double operate(const string& op, istringstream& iss, vector<Variable>& v)
{   
    double value;
    if (op == "+")
        value = perform_addition(iss, v);
    else if (op == "-")
        value = perform_subtraction(iss, v);
    else if (op == "/")
        value = perform_division(iss, v);
    else if(op == "*")
        value = perform_multiplication(iss, v);
    else if (is_number(op))
        value = atof(op.c_str());
    else
        value = get_variable(op, v);

    return value;
}

void define_new_var(vector<Variable>& v, istringstream& iss)
{
    string name;
    iss >> name;

    string temp;
    iss >> temp;

    double value = operate(temp, iss, v);

    v.push_back(Variable(name, value));
}


Комментарии
4 ответа

#include <iostream>
#include <vector>
#include <string>
#include <sstream>
#include <cstdlib> // mmocny: I needed to add this to use atof
#include <functional>

using namespace std;

//----------------------------------

class Variable
{
public:
Variable(const string& name, double val)
: name_(name), val_(val) // mmocny: Use initializer lists
{
}

// mmocny: get_* syntax is less common in C++ than in java etc.
const string& name() const { return name_; } // mmocny: Don't mark as inline (they already are, and its premature optimization)
double val() const { return val_; } // mmocny: Again, don't mark as inline
private:
string name_; // mmocny: suggest renaming name_ or _name: Easier to spot member variables in method code, and no naming conflicts with methods
double val_;
};

//----------------------------------

// mmocny: Replace print_* methods with operator<< so that other (non cout) streams can be used.
// mmocny: Alternatively, define to_string()/str() methods which can also be piped out to different streams
std::ostream & operator<<(std::ostream & out, Variable const & v)
{
return out << v.name() << ", " << v.val() << endl;
}

std::ostream & operator<<(std::ostream & out, vector<Variable> const & v)
{
for (vector<Variable>::const_iterator it = v.begin(), end = v.end(); it != end; ++it ) // mmocny: Use iterators rather than index access
{
out << *it << endl;
}
return out;
}

//----------------------------------

double get_variable(const string& op, vector<Variable>& v)
{
// mmocny: instead of using a vector<Variable> you should be using a map/unordered_map<string,double> and doing a key lookup here
int size = v.size();
for (int i = 0; i < size; i++)
{
if (op == v[i].name())
return v[i].val();
}
// mmocny: what do you do if you don't find the variable?
throw std::exception(); // mmocny: You should do something better than throw a generic exception()
}

//----------------------------------

bool is_number(const string& op)
{
// mmocny: someone else already mentioned: what if op is empty?
int char_to_int = op[0];
// mmocny: couple notes here:
// 1) chars are actually numbers you can reference directly, and not need "magic" constants
// 2) functions in the form "if (...) return true; else return false;" should just be reduced to "return (...);"
// 3) is_number functionality already exists in libc as isdigit()
// 4) long term, you are probably going to want to improve this function.. what about negative numbers? numbers in the form .02? etc..
//return (char_to_int >= '0' && char_to_int <= '9');
return isdigit(char_to_int);
}

//----------------------------------

// mmocny: replace istringstream with istream
// mmocny: you only need to predeclare this one function
double operate(const string& op, istream& in, vector<Variable>& v);

//----------------------------------
/*
* mmocny: All of your perform_* functions have identical code other than the operator being used.
* You can turn all of these functions into a single function template where you pass the operator to be used.
* Luckily, <functional> already has plus minus multiplies divides function objects (functors)
*/

template< class Operator >
double perform_action(istream& in, vector<Variable>& v, Operator op)
{
string left;
in >> left;

double result = operate(left, in, v); // mmocny: This is a big one: for correctness, you must calculate result of left BEFORE you read right

string right;
in >> right;

return op(result, operate(right, in, v));
}

//----------------------------------

double operate(const string& op, istream& in, vector<Variable>& v)
{
double value;
if (op == "+")
value = perform_action(in, v, plus<double>());
else if (op == "-")
value = perform_action(in, v, minus<double>());
else if(op == "*")
value = perform_action(in, v, multiplies<double>());
else if (op == "/")
value = perform_action(in, v, divides<double>());
else if (is_number(op))
value = atof(op.c_str()); // mmocny: consider using boost::lexical_cast<>, or strtod (maybe)
else
value = get_variable(op, v);

return value;
}

//----------------------------------

void define_new_var(vector<Variable>& v, istream& in)
{
string name;
in >> name;

string temp;
in >> temp;

double value = operate(temp, in, v);

v.push_back(Variable(name, value));
}

//----------------------------------

int main()
{
cout << endl << "LeRPN Programming Language" << endl;

vector<Variable> v;

string temp;
while (cin)
{
cout << endl << "> ";

getline(cin, temp);

if (temp.empty()) // mmocny: This also handles the case of CTRL+D
continue;

istringstream iss(temp);
string op;
iss >> op;

if (op == "quit")
break;
else if (op == "def")
define_new_var(v, iss);
else if (op == "show_vars")
std::cout << v << std::endl;
else
cout << endl << operate(op, iss, v) << endl;
}
}

Это мои изменения, с комментариями в коде. Я хотел бы сделать больше изменений, но достаточно это сейчас :)

Заметил одно большое изменение: у вас серьезная ошибка в правильности вашего perform_* функции. Не то, чтобы я проверил мой измененный код выше для всех случаев, но исходный код был с всегда неправильно для любых вложенных расчетов.

5
ответ дан 3 мая 2011 в 07:05 Источник Поделиться

Как незначительные сторону, я переименовал его в ПН , а не РПН. Форме ты дал (с операторами предыдущих операндов) является польской нотации как Ян Лукасевич изобрел его. РПН-это когда ты обратное, что и у первого операнда, за соответствующим оператором.

А почему они решили назвать то, что РПН: потому что английского языка было достаточно времени выяснить, что его фамилия произносится примерно как "Wookashayvitch", не говоря уже, пытаясь выяснить, как сказать что-то недопонял.

В любом случае, я думаю, я мог бы писать код вроде этого:

#include <iostream>
#include <vector>
#include <string>
#include <sstream>
#include <map>
#include <iterator>

using namespace std; // really would *not* normally do this, but...

void define_var(map<string, int> &v, istringstream& iss) {
std::string name;
int value;
iss >> name >> value;
v[name] = value;
}

int do_op(char op, int val1, int val2) {
switch (op) {
case '+': return val1 + val2;
case '-': return val1 - val2;
case '*': return val1 * val2;
case '/': return val1 / val2;
default:
string error("Unknown operator: ");
error += op;
throw runtime_error(error);
}
}

bool isoperator(char ch) {
return ch == '+' || ch == '-' || ch == '*' || ch == '/';
}

char getop(istream &is) {
char ch;
while (isspace(ch = is.peek()))
is.get(ch);
ch = is.peek();
return ch;
}

int eval(istream &is, map<string, int> const &v) {
// evaluate an expression. It consists of:
// an operator followed by operands, or
// a number, or
// a variable.
//

char ch = getop(is);

if (isoperator(ch)) {
is.get(ch);
int val1 = eval(is, v);
int val2 = eval(is, v);
return do_op(ch, val1, val2);
}
if (isdigit(ch)) {
int val;
is >> val;
return val;
}

string var_name;
is >> var_name;
map<string, int>::const_iterator p = v.find(var_name);
if (p==v.end()) {
string problem("Unknown variable: ");
problem +=var_name;
throw runtime_error(problem.c_str());
}
return p->second;
}

// used only for dumping out the variables.
namespace std {
ostream &operator<<(ostream &os, pair<string, int> const &v) {
return os << v.first << ": " << v.second;
}
}

int main() {
cout << endl << "LePN Programming Language" << endl;

map<string, int> v;

string temp;
cout << endl << "> ";
while (getline(cin, temp)) {
istringstream iss(temp);

string op;
iss >> op;

if (op == "quit")
break;
else if (op == "def")
define_var(v, iss);
else if (op == "show_vars")
std::copy(v.begin(), v.end(), ostream_iterator<pair<string, int> >(cout, "\n"));
else {
// Technically, this isn't right -- it only ungets one
// character, not the whole string.
// For example, this would interpret "this+ 2 3" as "+ 2 3"
// and give 5 instead of an error message. Shouldn't affect
// correct input though.
//
iss.unget();
cout << endl << eval(iss, v) << endl;
}
cout << endl << "> ";
}
}

3
ответ дан 3 мая 2011 в 11:05 Источник Поделиться


  1. двойной get_variable(константные строки& ОП, вектор& в)-
    должен возвращать значение по умолчанию (0.0), например, если ОП пустой. Кроме того, вы можете показать сообщение об ошибке.

  2. двойной работы(константные строки& ОП, istringstream& МКС, вектор& в) -
    Всегда инициализировать переменные, как это:

    double value(0.0); OR
    double value = 0.0;

    и вы должны проверить этот ОП в вектор, если его не существует - показать ошибку.


  3. пустота define_new_var(вектор& в istringstream& МКС)-
    если переменная уже существует, установить новое значение или показать ошибку.

  4. боол is_number(константные строки& ОП)

    int char_to_int = op[0];

    если ОП пустой.


  5. Для этой функции:

    double perform_addition(istringstream& iss, vector<Variable>& v);
    double perform_subtraction(istringstream& iss, vector<Variable>& v);
    double perform_division(istringstream& iss, vector<Variable>& v);
    double perform_multiplication(istringstream& iss, vector<Variable>& v);

    Определить общие функции, которые получают влево и вправо.

    Я думаю, что вы можете определить некоторые перечисленияS и функции, такой:

    double perform_operator(istringstream& iss, vector<Variable>& v, OperatorType type)
    {
    std::string lhs, rhs;
    GetValues(iss, lhs, rhs); // fill them
    switch(type)
    {
    case Minus
    {
    return operate(left, iss, v) - operate(right, iss, v);
    }
    break;
    }
    }

2
ответ дан 3 мая 2011 в 07:05 Источник Поделиться

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

1
ответ дан 3 мая 2011 в 06:05 Источник Поделиться