Ograniczenie Copy&Paste i poprawa klasy

Hej,

zanim zaczniecie mnie linczować, to z góry przepraszam i jestem świadomy, że kod, które przedstawię jest daleki od doskonałości.
To mów pierwszy "większy" projekt z c++ okienkowo, w dodatku na uczelnię. Zależało mi po prostu na tym, żeby to działało.

Teraz, po sesji chcę go poprawić abym nie musiał się go wstydzić.

Jest to "parser wyrażeń matematycznych oparty na ONP".

Szczególnie wstyd mi za metodę operation_check, ale czy jest jakiś inny sposób by zawrzeć warunki kiedy wyrażenie jest źle wprowadzone? Pewnie przy "parsowaniu" w metodzie InfixToPostfix, ale tam będzie ciężko je teraz wkręcić...

Słowem, czy jest szansa na poprawienie tej klasy czy lepiej napisać ją od nowa? Jest tu trochę copy&paste, od rana staram się to usunąć, przenieść do mniejszych funkcji.

Jakieś rady co z tym zrobić?

#include "onp.h"

using namespace std;

onp::onp()
{
    result = "";
    error = "";
    raport = "";
}

QString onp::InfixToPostfix(QString operation) throw(QString)
{
    stack <QChar> stos;
    size_t i=0;
    size_t y=operation.length();
    if(operation_check(operation))
    {
        error = "Błąd składniowy wyrażenia!\n\n"
                "Program nie rozpoznaje wyrażenia."
                "\nZapoznaj się z pomocą programu.";
        throw error;
    }
    while(i<y)
    {
        if(operation[i]>='0'&&operation[i]<='9')
        {
            while((operation[i]>='0'&&operation[i]<='9')||operation[i]=='.')
            {
                result.push_back(operation[i]);
                i++;
            }
        }
        result.push_back(" ");
        // jeśli stos nie pusty lub operacja to jakaś funkcja trygonometryczna 
        if(stos.size()>0||operation[i]=='s'||operation[i]=='c'||operation[i]=='t'||operation[i]=='l'||operation[i]=='!')
        {
            if((operation[i]=='-'&&((operation[i-1]>='0'&&operation[i-1]<='9')||(operation[i-1]==')')))
                    ||
                    operation[i]=='+')
            {
                while(stos.top()!='(')
                {
                    result.push_back(stos.top());
                    stos.pop();
                    result.push_back(" ");
                    if(stos.size()==0)
                    {
                        break;
                    }
                }
                stos.push(operation[i]);
                i++;
            }
            else if(operation[i]=='-'&&operation[i-1]=='(')
            {
                stos.push('~');
                i++;
            }
            else if((operation[i]=='/'||operation[i]=='*')
                    ||
                    ((operation[i-1]>='0'&&operation[i-1]<='9')&&operation[i]=='('))
            {
                if(stos.size())
                {
                    while(IfSmalerPriority(stos.top(),operation[i]))
                    {
                        result.push_back(stos.top());
                        stos.pop();
                        result.push_back(" ");
                        if(stos.size()==0)
                        {
                            break;
                        }
                    }
                    if((operation[i-1]>='0'&&operation[i-1]<='9')&&operation[i]=='(')
                    {
                        stos.push('*');
                        stos.push('(');
                        i++;
                    }
                    else
                    {
                        stos.push(operation[i]);
                        i++;
                    }
                }
            }
            else if(operation[i]=='^'||operation[i]==0x221A)//0x221A to unicode dla pierwiastka
            {
                if(stos.size())
                {
                    while(IfSmalerPriority(stos.top(),operation[i]))
                    {
                        result.push_back(stos.pop());
                        result.push_back(" ");
                        if(stos.size()==0)
                        {
                            break;
                        }
                    }
                }
                stos.push(operation[i]);
                i++;
            }
            else if(operation[i]=='!'&&operation[i+1]=='(')
            {
                i=i+1;
                stos.push('!');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='s'&&operation[i+1]=='i'&&operation[i+2]=='n'&&operation[i+3]=='(')
            {
                i=i+3;
                stos.push('s');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='c'&&operation[i+1]=='o'&&operation[i+2]=='s'&&operation[i+3]=='(')
            {
                i=i+3;
                stos.push('c');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='c'&&operation[i+1]=='t'&&operation[i+2]=='g'&&operation[i+3]=='(')
            {
                i=i+3;
                stos.push('k');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='l'&&operation[i+1]=='o'&&operation[i+2]=='g'&&operation[i+3]=='(')
            {
                i=i+3;
                stos.push('L');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='t'&&operation[i+1]=='a'&&operation[i+2]=='n'&&operation[i+3]=='(')
            {
                i=i+3;
                stos.push('t');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='l'&&operation[i+1]=='n'&&operation[i+2]=='(')
            {
                i=i+2;
                stos.push('l');
                stos.push('(');
                i++;
            }
            else if(operation[i]=='(')
            {
                stos.push(operation[i]);
                i++;
            }
            else if(operation[i]==')')
            {
                if(stos.size())
                {
                    while(stos.top()!='('&&stos.size()>0)
                    {
                        result.push_back(stos.top());
                        result.push_back(" ");
                        stos.pop();
                    }
                }
                if(stos.size())
                {
                    stos.pop();
                }
                i++;
            }
            else
                i++;
        }
        else
        {
            stos.push(operation[i]);
            i++;
        }
    }
    while(stos.size())
    {
        result.push_back(stos.top());
        result.push_back(" ");
        stos.pop();
    }
    return result;
}

int onp::GetOperatorWeight(QChar op)
{
    int weight=0;
    if(op=='+'||op=='-') weight=1;
    else if(op=='*'|| op=='/') weight=2;
    else if(op=='^'|| op==0x221A) weight=3;
    else if(op=='s'|| op=='c'||op=='t'|| op=='k'||op=='l'|| op=='L' || op=='!') weight=4;

    return weight;
}

bool onp::IfSmalerPriority(QChar op1, QChar op2)
{
    int op1weight=GetOperatorWeight(op1);
    int op2weight=GetOperatorWeight(op2);

    if(op1weight>=op2weight)
        return true;

    return false;
}

double onp::ComputePostfixOperation(QString string) throw(QString)
{
    stack<double> stos;
    QString pom2="";
    int i=0;
    double a=0,b=0,pom=0;

    while(string.length()>i)
    {
        if(string[i]>='0'&&string[i]<='9')
        {
            while((string[i]>='0'&&string[i]<='9')||string[i]=='.')
            {
                pom2.push_back(string[i]);
                i++;
            }
            stos.push(pom2.toDouble());
            pom2="";
        }
        if(string[i]=='+')
        {
            if(stos.size()>0)
            {
                a=stos.pop();
                b=stos.pop();
                pom=b+a;
                stos.push(pom);
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='-')
        {
            if(stos.size()==1)
            {
                a=stos.pop();
                stos.push(-1*a);
                i++;
            }
            else
            {
                if(stos.size()>0)
                {
                    a=stos.pop();
                    b=stos.pop();
                    pom=b-a;
                    stos.push(pom);
                }
                else
                {
                    error="Błąd operatorów lub wyrażenia!";
                    throw error;
                }
                i++;
            }
        }
        else if(string[i]=='*')
        {
            if(stos.size()>0)
            {
                a=stos.pop();
                b=stos.pop();
                pom=b*a;
                stos.push(pom);
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='/')
        {
            if(stos.size()>0)
            {
                a=stos.pop();
                b=stos.pop();
                if(a!=0)
                {
                    pom=b/a;
                    stos.push(pom);
                }
                else
                {
                    error="Dzielenie przez zero!";
                    throw error;
                }
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='^')
        {
            if(stos.size()>0)
            {
                a=stos.pop();
                b=stos.pop();
                pom=pow(b,a);
                stos.push(pom);
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]==0x221A)
        {
            if(stos.size()>0)
            {
                if(stos.top()>=0)
                {
                    a=stos.pop();
                    pom=sqrt(a);
                    stos.push(pom);
                }
                else
                {
                    error="Pierwiastkowanie liczby ujemnej!";
                    throw error;
                }
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='~')
        {
            if(stos.size()>0)
            {
                a=stos.pop();
                pom=-1*a;
                stos.push(pom);
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='s')
        {
            a=stos.pop();
            double rad=a*M_PI/180;
            pom=sin(rad);
            stos.push(pom);
            i++;
        }
        else if(string[i]=='c')
        {
            a=stos.pop();
            double rad=a*M_PI/180;
            pom=cos(rad);
            stos.push(pom);
            i++;
        }
        else if(string[i]=='t')
        {
            a=stos.pop();
            if(a!=90)
            {
                double rad=a*M_PI/180;
                pom=tan(rad);
                stos.push(pom);
                i++;
            }
            else
            {
                error="Tangens z 90 stopni nie istnieje!";
                throw error;
            }
        }
        else if(string[i]=='k')
        {
            a=stos.pop();
            if(a!=0&&a!=180)
            {
                double rad=a*M_PI/180;
                pom=pow(tan(rad),-1);
                stos.push(pom);
                i++;
            }
            else
            {
                error="Cotangens z 180 lub 0 stopni nie istnieje!";
                throw error;
            }
        }
        else if(string[i]=='L')
        {
            a=stos.pop();
            pom=log10(a);
            stos.push(pom);
            i++;
        }
        else if(string[i]=='l')
        {
            if(stos.size())
            {
                a=stos.pop();
                pom=log(a);
                stos.push(pom);
            }
            else
            {
                error="Błąd operatorów lub wyrażenia!";
                throw error;
            }
            i++;
        }
        else if(string[i]=='!')
        {
            a=stos.pop();
            if(a>=0)
            {
                mathematical nowy;
                pom=nowy.factorial((int)a);
                stos.push(pom);
                i++;
            }
            else
            {
                error="Silnie liczymy z liczb nieujemnych!";
                throw error;
            }
        }
        else if(string[i]==' ')
        {
            i++;
        }
        else
        {
            error="Nieznany znak lub błąd wyrażenia!";
            throw error;
        }
    }
    return stos.pop();
}

int onp::operation_check(QString operation)
{
    int z=operation.length();
    bool flaga=false;
    QString dozwolone="sin().+-*/^cotag0123456789l!";
    for(int i=0; i<z; i++)
    {
        for(int y=0; y<dozwolone.length(); y++)
        {
            if((operation[i]==dozwolone[y])||(operation[i]==0x221A))
            {
                flaga=false;
                break;
            }
            else
                flaga=true;
        }
        if((operation[i]=='-'&&operation[i+1]=='-') //jeśli warunek spełniony oznacza to, że wyrażenie zawiera błąd składniowy
                ||
                (operation[i]=='-'&&operation[i+1]=='+') //to tutaj i poniżej to masakra, znacie jakiś sposób jak te warunki lepiej ująć?
                ||
                (operation[i]=='-'&&operation[i+1]=='/')
                ||
                (operation[i]=='-'&&operation[i+1]=='^')
                ||
                (operation[i]=='-'&&operation[i+1]==')')
                ||
                (operation[i]=='-'&&operation[i+1]=='.')
                ||
                (operation[i]=='+'&&operation[i+1]=='-')
                ||
                (operation[i]=='+'&&operation[i+1]=='+')
                ||
                (operation[i]=='+'&&operation[i+1]=='/')
                ||
                (operation[i]=='+'&&operation[i+1]=='^')
                ||
                (operation[i]=='+'&&operation[i+1]==')')
                ||
                (operation[i]=='+'&&operation[i+1]=='.')
                ||
                (operation[i]=='*'&&operation[i+1]=='-')
                ||
                (operation[i]=='*'&&operation[i+1]=='+')
                ||
                (operation[i]=='*'&&operation[i+1]=='/')
                ||
                (operation[i]=='*'&&operation[i+1]=='^')
                ||
                (operation[i]=='*'&&operation[i+1]==')')
                ||
                (operation[i]=='*'&&operation[i+1]=='.')
                ||
                (operation[i]=='/'&&operation[i+1]=='-')
                ||
                (operation[i]=='/'&&operation[i+1]=='+')
                ||
                (operation[i]=='/'&&operation[i+1]=='/')
                ||
                (operation[i]=='/'&&operation[i+1]=='^')
                ||
                (operation[i]=='/'&&operation[i+1]==')')
                ||
                (operation[i]=='/'&&operation[i+1]=='.')
                ||
                (operation[i]=='^'&&operation[i+1]=='-')
                ||
                (operation[i]=='^'&&operation[i+1]=='+')
                ||
                (operation[i]=='^'&&operation[i+1]=='/')
                ||
                (operation[i]=='^'&&operation[i+1]=='^')
                ||
                (operation[i]=='^'&&operation[i+1]==')')
                ||
                (operation[i]=='^'&&operation[i+1]=='.')
                ||
                (operation[i]=='('&&operation[i+1]=='+')
                ||
                (operation[i]=='('&&operation[i+1]=='/')
                ||
                (operation[i]=='('&&operation[i+1]=='^')
                ||
                (operation[i]=='('&&operation[i+1]==')')
                ||
                (operation[i]=='('&&operation[i+1]=='.')
                ||
                (operation[i]=='^'&&operation[i+1]=='-')
                ||
                (operation[i]=='^'&&operation[i+1]=='+')
                ||
                (operation[i]==0x221A&&operation[i+1]=='/')
                ||
                (operation[i]==0x221A&&operation[i+1]=='^')
                ||
                (operation[i]==0x221A&&operation[i+1]==')')
                ||
                (operation[i]==0x221A&&operation[i+1]=='.')
                ||
                (operation[i]==0x221A&&operation[i+1]=='+')
                ||
                (operation[i]==0x221A&&operation[i+1]=='-')
                ||
                (operation[i]=='!'&&operation[i+1]=='/')
                ||
                (operation[i]=='!'&&operation[i+1]=='^')
                ||
                (operation[i]=='!'&&operation[i+1]==')')
                ||
                (operation[i]=='!'&&operation[i+1]=='.')
                ||
                (operation[i]=='!'&&operation[i+1]=='+')
                ||
                (operation[i]=='!'&&operation[i+1]=='-')
                ||
                (operation[i]=='l'&&operation[i+1]=='n'&&operation[i]!='(')
                ||
                (operation[i]=='t'&&operation[i+1]=='g'&&operation[i]!='(')
                ||
                (operation[i]=='!'&&operation[i+1]!='(')
                ||
                (operation[i]=='s'&&operation[i+1]=='i'&&operation[i+2]=='n'&&operation[i+3]!='(')
                ||
                (operation[i]=='c'&&operation[i+1]=='o'&&operation[i+2]=='s'&&operation[i+3]!='(')
                ||
                (operation[i]=='c'&&operation[i+1]=='t'&&operation[i+2]=='g'&&operation[i+3]!='(')
                ||
                (operation[i]=='l'&&operation[i+1]=='o'&&operation[i+2]=='g'&&operation[i+3]!='(')
                )
        {
            flaga=true;
        }

        if(flaga)
            return 1;
    }
    return 0;
}
 

wiem, ze wiesz ze słabej jakości ale nadal

wystarczylo pogooglowac
http://www.algorytm.org/algorytmy-arytmetyczne/odwrotna-notacja-polska/onp-1-c.html
a to i tak nie jest najlepszy kod. Gdzies @Patryk27 fajnego gotowca kiedys wklejal (jezlei dobrze pamietam)

Ten algorytm który wstawiłeś nie działa, poza tym on dodaje, odejmuje, mnoży i dzieli, a mój ma znacznie więcej opcji, przez co i kod stał się kilkakrotnie bardziej zawiły.
Nie znalazłem w internecie podobnego projektu. Są, ale takie, które oferują mniej opcji. Tutaj użytkownika ogranicza tylko to, że powinien wpisać względnie poprawne wyrażenie.

I nie bardzo zależy mi na gotowcach, na googlach się znam. Chodzi mi o to, abyście mnie nakierowali co dalej z tą klasą - pisać od nowa czy da się ją jakoś zminimalizować, poprawić?

Dlaczego operation_check() a nie OperationCheck() skoro wszystkie inne metody są z CamelCase?

Jak widzę takie ify to pierwsza (lecz niekoniecznie najlepsza!) myśl to stworzenie jakiegoś zbioru reguł. W tym przypadku mogłoby to wyglądać tak

set<string> forbidden = { "--", "-+", "-/", "-^", .... };
...
if (forbidden.find(operation.substr(i, 2)) != forbidden.end())
{
    cout << "oops I did it again";
}

Ja na Twoim miejscu bym napisał od nowa.

Z grubsza to
Podziel wyrażenie na tokeny, czyli z np. 20+1/sin(45) robisz wektor tokenów '20', '+', '1', '/', 'sin', '(', '45', ')'. Do reprezentacji tokena użyj jakiejś struktury, która będzie trzymała jego wartość i typ (to tylko przykładowe, może wpadniesz na lepszą reprezentację). Jeżeli przy parsowaniu wpadniesz na jakiś niepoprawny token to wiesz, że wyrażenie jest niepoprawne. Dodatkowo algorytm załatwi za Ciebie kontrolę nawiasów. https://pl.wikipedia.org/wiki/Odwrotna_notacja_polska#Algorytm_konwersji_z_notacji_infiksowej_do_ONP

Jeśli nie ma więcej symboli do przeczytania, zdejmuj wszystkie symbole ze stosu (jeśli jakieś są) i dodawaj je do kolejki wyjścia. (Powinny to być wyłącznie operatory, jeśli natrafisz na jakiś nawias oznacza to, że nawiasy zostały źle umieszczone.)

Co do prostego sprawdzania co po czym może być użyłbym mapy

std:map<TokenType, std::vector<TokenType>> 

I lecisz

bool valid( TokenType current, TokenType next )
{
	const auto &validTokens = map[current];
	
	return std::find( validTokens.begind(),
	                  validTokens.end(),
	                  next ) != validTokens.end();
}

Znów, to tylko przykład, pewnie jest lepszy sposób.

Mając taki wektor tokenów na prawdę łatwiej będzie ci zrobić z infix onp i później onp obliczyć.

Zainteresuj się boost.spirit i napisz do tego normalną gramatykę.

Tutaj gotowce z brzegu:
http://boost-spirit.com/distrib/spirit_1_8_2/libs/spirit/example/fundamental/more_calculators/rpn_calc.cpp
http://agentzlerich.blogspot.com/2011/06/using-boost-spirit-21-to-evaluate.html

twonek napisał(a):

Dlaczego operation_check() a nie OperationCheck() skoro wszystkie inne metody są z CamelCase?

operation_check - pisałem w nocy, fakt. Poprawione ;)

twonek napisał(a):

Jak widzę takie ify to pierwsza (lecz niekoniecznie najlepsza!) myśl to stworzenie jakiegoś zbioru reguł. W tym przypadku mogłoby to wyglądać tak

set<string> forbidden = { "--", "-+", "-/", "-^", .... };
...
if (forbidden.find(operation.substr(i, 2)) != forbidden.end())
{
    cout << "oops I did it again";
}

Hmm, faktycznie, dobry pomysł. Znacznie mi to ograniczy tą "drabinkę". Zaraz to napisze. :)

Dzięki za uwagi wszystkim, zaraz się za to biorę. Wstawię efekty. :)