Dzzirt / OOP

0 stars 0 forks source link

Замечания по InvertMatrix #7

Open alexey-malov opened 8 years ago

alexey-malov commented 8 years ago
double Get3dMatrixDeterminant(Matrix const& matrix)
{
    if (matrix.size() != 3 || matrix[0].size() != 3)
    {
        throw invalid_argument("Matrix should be 3x3");
    }
    double determinant = matrix[0][0] * matrix[1][1] * matrix[2][2] +
        matrix[2][0] * matrix[0][1] * matrix[1][2] +
        matrix[1][0] * matrix[2][1] * matrix[0][2] -
        matrix[2][0] * matrix[1][1] * matrix[0][2] -
        matrix[0][0] * matrix[2][1] * matrix[1][2] -
        matrix[1][0] * matrix[0][1] * matrix[2][2];
    return determinant;
}

вектор векторов - не самая лучшая реализация для матрицы, т.к. каждая строка может иметь произвольное количество строк. то, что у матрицы 3 строки и первая строка состоит из 3-х столбцов, не является достаточным основанием считать, что остальные 2 строки также содержат 3 столбца Лучше использовать std::array для хранения матриц 3 * 3 и 2 * 2 Либо в структуру положить двумерный массив Тем более, что дискриминант ты все равно считаешь для матрицы 3*3

alexey-malov commented 8 years ago
double GetMinorByPos(Matrix const& matrix, std::pair<size_t, size_t> const& pos)
{
    if (matrix.size() != 3 || matrix[0].size() != 3)
    {
        throw invalid_argument("Matrix should be 3x3");
    }

аналогично

alexey-malov commented 8 years ago
Matrix ToAlgAdditions3dMatrix(Matrix & matrix)
{
    if (matrix.size() != 3 || matrix[0].size() != 3)
    {
        throw invalid_argument("Matrix should be 3x3");
    }

и тут тоже

Алгебраическое дополнение по английски будет Cofactor

alexey-malov commented 8 years ago

Тест не проходит

1>  d:/teaching/2016/kuzin/oop/lab1/invert/inverttest/invertfunctionstest.cpp(70): error in "InvertProcess": check Get3dMatrixFromFile("input.txt") == matrix3d failed
alexey-malov commented 8 years ago
            int lineLength = static_cast<int>(strm.tellg());
            if (lineLength != -1)
            {
                auto line = strm.str();
                if (!ContainsAfterIncluding(line, ' ', lineLength) &&
                    !ContainsAfterIncluding(line, '\t', lineLength))
                {
                    throw invalid_argument("Input file should contain 3x3 matrix");
                };
            }

можно заменить на более элегантное:

            if (getline(strm, line) && any_of(line.begin(), line.end(), isspace))
            {
                throw invalid_argument("Input file should contain 3x3 matrix");
            }
alexey-malov commented 8 years ago
int main(int argc, char* argv[])
{
    std::string fileName = argv[1];
    PrintMatrix(Invert3dMatrix(Get3dMatrixFromFile(fileName)));
    return 0;
}

-Не зная броду, не лезь в воду. Следует проверить argc прежде чем использовать argv -Если используешь исключения, не забудь их перехватывать

alexey-malov commented 8 years ago
    Matrix matrix = { { {}, {}, {} }, { {}, {}, {} }, { {}, {}, {} } };

Тебе не кажется этот код несколько странным и неуклюжим?

alexey-malov commented 8 years ago

Работа пока принята с коэффициентом 0,5

Dzzirt commented 8 years ago

Исправил

alexey-malov commented 8 years ago

alexey-malov commented 8 years ago
Matrix GetCofactor3dMatrix(Matrix & matrix)
{
    matrix[0][1] *= -1;
    matrix[1][0] *= -1;
    matrix[1][2] *= -1;
    matrix[2][1] *= -1;
    return matrix;
}

Почему модифицируется оригинальная матрица? Принимай тогда уж по значению Функция в действительности на находит матрицу алгебраических дополнений (хотя и используется в процессе нахождения). Почитай определение: http://www.mathwords.com/c/cofactor_matrix.htm

alexey-malov commented 8 years ago
Matrix Multiply3dMatrixWithNum(Matrix & matrix, double value)
{
    for (size_t row = 0; row < matrix.size(); row++)
    {
        for (size_t col = 0; col < matrix[0].size(); col++)
        {
            matrix[row][col] *= value;
        }
    }
    return matrix;
}

Странно модифицировать входной аргумент и возвращать результат по значению. Тогда уж лучше принимать аргумент по значению

alexey-malov commented 8 years ago

k=0,8