Linear quation system solver

Question

I've done an equation system that takes as input the equations (you choose the number of equations), and gives as output the values of the unknowns.

I splitted the code in four files:

-equation class(move all to left member and acquire values)

-matrix class(find determinants)

-equations system class(craft matrices to find all the determinants)

-the main() asks for the equations

main.cpp

#include "EquationSystem.h"

int main()
{

    vector<string> equations;
    string nEq;

    cout << "Quante equazioni? ";

    getline(cin, nEq);
    stringstream ss;
    ss << nEq;
    int n;
    if (ss >> n)
    {
        for (int i = 0; i < n; i++)
        {
            string eq;
            cout << "Inserisci l'equazione numero " << i + 1 << " : ";
            getline(cin, eq);
            equations.push_back(eq);
        }
        EquationSystem system(equations);
        system.solve();
    }
    else cout << "ERROR\n";
    system("pause");
    return 0;
}

Equation.h

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

using namespace std;

class Equation
{
private:
    string equation;
    string leftMember, rightMember;
    map<string, double> unknowns;
    void elaborate(string &member, bool isLeft)
    {
        int unkCount = count_if(member.begin(), member.end(), isalpha);
        int prevUnkPos = 0;
        for (int i = 0; i < unkCount; i++)
        {
            int unkPos = find_if(member.begin() + prevUnkPos, member.end(), isalpha) - member.begin();
            if (!isdigit(member[unkPos - 1]) && !isalpha(member[unkPos - 1]))
                member.insert(unkPos, "1");
            prevUnkPos = find_if(member.begin() + prevUnkPos, member.end(), isalpha) - member.begin() + 1;
        }
        cout << member << endl;

        this->parse(member, isLeft);
    }
    void parse(string &member, bool isLeft)
    {
        istringstream iss(member);
        while ((int)iss.tellg() != EOF)
        {
            double coef, pow;
            char let, powSign;

            iss >> coef;
            stringstream ss;
            while (isalpha(iss.peek()))
            {
                iss >> let;
                ss << let;
                if (iss.peek() == '^')
                {
                    iss >> powSign >> pow;
                    ss << powSign << pow;
                }
            }
            if (!isLeft)
                coef *= -1;
            unknowns[ss.str()] += coef;
        }
    }
public:
    friend class EquationSystem;
    Equation(string equation)
    {
        this->equation = equation;
        if (this->equation.find_first_of("=") == string::npos)
            cout << "NOT AN EQUATION\n";
        else
        {
            this->leftMember = this->equation.substr(0, this->equation.find_first_of("="));
            this->rightMember = this->equation.substr(this->equation.find_first_of("=") + 1, this->equation.length());
            this->elaborate(leftMember, 1);
            this->elaborate(rightMember, 0);
        }
    }

};

EquationSystem.h

#include "Equation.h"
#include "Matrix.h"
#include <set>

class EquationSystem
{
private:
    vector<Equation> equations;
    vector<string> unknowns;
    map<string, double> solutions;
public:
    EquationSystem(vector<string> equation)
    {

        set<string> unk;
        for (auto i : equation)
            this->equations.push_back(Equation(i));
        for (auto i : equations)
        {
            for (auto j : i.unknowns)
                unk.insert(j.first);
        }
        unknowns.assign(unk.rbegin(), unk.rend());
    }
    void solve()
    {
        double det = toMatrix("").det();
        for_each(unknowns.begin(), unknowns.end() - 1, [this, det](string unknown) {solutions[unknown] = toMatrix(unknown).det() / det; });
        for (auto i : solutions)
        {
            cout << i.first << " e' uguale a " << i.second << endl;
        }
    }
    Matrix toMatrix(string toChange)
    {
        vector<vector<double>> nums(equations.size(), vector<double>(unknowns.size() - 1));
        for (int i = 0; i < equations.size(); i++)
        {
            int j = 0;
            for_each(unknowns.begin(), unknowns.end() - 1, [this, &nums, i, &j, toChange](string unknown) {if (unknown == toChange) nums[i][j] = -equations[i].unknowns[""]; else nums[i][j] = equations[i].unknowns[unknown];   j++; });
        }
        Matrix mat(nums);
        cout << "Il determinante ";
        if (!(toChange == ""))
            cout << "di " << toChange << " ";
        cout << "e' \n";
        cout << mat << "        = " << mat.det() << endl << endl;
        return mat;
    }
};

Matrix.h

#include <vector>
#include <iomanip>

using namespace std;

class Matrix
{
private:
    vector<vector<double>> mat;
    Matrix algComp(int x, int y)
    {
        Matrix algComp(this->mat);

        for_each(algComp.mat.begin(), algComp.mat.end(), [x](vector<double> &row) {row.erase(row.begin() + x); });
        algComp.mat.erase(algComp.mat.begin() + y);

        return algComp;

    }
public:
    Matrix(vector<vector<double>> &matrix)
    {
        this->mat = matrix;
    }
    double det()
    {
        double det = 0;
        if (mat.size() > 1)
        {
            for (int i = 0; i < mat.size(); i++)
            {
                det += pow(-1, i)*mat[0][i] * algComp(i, 0).det();
            }
            return det;
        }
        else return mat[0][0];
    }
    vector<double> &operator[](int n)
    {
        return mat[n];
    }
    friend ostream &operator<<(ostream &os, Matrix m)
    {
        for (int i = 0; i < m.mat.size(); i++)
        {
            for (int j = 0; j < m.mat[i].size(); j++)
            {
                os << right << setw(3) << m[i][j] << " ";
            }
            if (!(i == m.mat.size() - 1))
                os << endl;
        }
        return os;
    }
};

How does it look like?

Answer 1

Algorithm, while theoretically correct, from the computational standpoint is arguably the worst possible. Its time complexity is exponential, and it is numerically unstable.

Naming is very confusing. For example,

Matrix algComp(int x, int y)
{
    Matrix algComp(this->mat);
    ....

is very hard to follow that algComp is a matrix, and not a recursive call. It is also very unclear what algComp, toChange, elaborate stand for.

One-liners like

        for_each(unknowns.begin(), unknowns.end() - 1, [this, &nums, i, &j, toChange](string unknown) {if (unknown == toChange) nums[i][j] = -equations[i].unknowns[""]; else nums[i][j] = equations[i].unknowns[unknown];   j++; });

are absolutely unreadable. At least, consider

        for_each(unknowns.begin(), unknowns.end() - 1,
            [this, &nums, i, &j, toChange](string unknown) {
               if (unknown == toChange)
                   nums[i][j] = -equations[i].unknowns[""];
               else
                   nums[i][j] = equations[i].unknowns[unknown];
               j++;
            });

Do not mix logic with UI. Computation utility methods shall not print anything.

PS: It is perfectly all right to inline logic into the class definition.

Answer 2

I'm not very familiar with C++, so I'll make only one suggestion: Try to never put logic into a header (.h or .hpp) file, use source (.cpp) files for that purpose. Try to leave header files only for declarations and macros. In fact, since #include is only text inclusion, you can include .cpp files too if you absolutely need to include logic and definitions. Although that's not common in small projects like this, in larger projects it's more common (thanks @Mast!).

Also, one more point, try not use using namespace std in your code, it might cause namespace collision among other bad things (see this SO post: Why is using namespace std considered bad practice). Try to use qualified names instead.

One more thing I found: EquationSystem.h and main.cpp have output in Spanish, while the others are in English. Has code been integrated from somewhere? Try not to have inconsistencies like this in code, especially in the UI.