RattleInGlasses / ps_oop

0 stars 0 forks source link

Замечания по программе инвертирования матриц #3

Open alexey-malov opened 9 years ago

alexey-malov commented 9 years ago
void GetMinorMatrix(double const matrix[3][3], double minorMatrix[3][3])
{
    double minor[2][2];
    for (int i = 0; i < 3; i++)
    for (int j = 0; j < 3; j++)
    {
        GetMinor(matrix, j, i, minor);
        minorMatrix[i][j] = pow(-1.0, i + j) * Determinant(minor);
    }
}

Лучше внешний цикл снабдить {}, заключающими внутренний

alexey-malov commented 9 years ago

Чтобы постоянно не писать double matrix[3][3] можно сделать typedef: typedef double Matrix3x3[3][3]; аналогично объявить Matrix2x2

alexey-malov commented 9 years ago

Массив можно вернуть по значению, а также передать по значению в функцию (если надо), если обернуть его в структуру:

struct Matrix3x3
{
  double items[3][3];
};
alexey-malov commented 9 years ago
// Multiply matrix by number
void NumMultMatrix(double const inputMatrix[3][3], double number, double resultMatrix[3][3])

Лучше дать имя вроде: MultMatrixByScalar

alexey-malov commented 9 years ago
    double matrixDet = Determinant(matrix);
    if (matrixDet == 0)

Числа с плавающей запятой лучше сравнивать приблизительно - разность между ними не должна превышать некого значения epsilon. Можно взять DBL_EPSILON за основу (float.h)

alexey-malov commented 9 years ago

exit-ов быть не должно

alexey-malov commented 9 years ago

Лучше ввести доп. функцию, выполняющую считывание матрицы из файла с указанным именем Функцию main хотелось бы видеть примерно такой:

if (!CheckArgs(argc))
{
  return 2;
}

Matrix3x3 mat;
if (ReadMatrixFromFile(argv[1], mat))
{
  Matrix3x3 result;
  if (InvertMatrix(mat, result))
  {
    PrintMatrix(result);
    return 0;
  }
  else
  {
     printf("Matrix is Singular");
     return 1;
  }
}
else
{
    printf("Failed to read matrix from file");
}

Либо при использовании исключений:

try
{
CheckArguments(argc);
Matrix3x3 matrix = ReadMatrixFromFile(argv[1]);
PrintMatrix(InvertMatrix(matrix));
}
catch (const exception& e)
{
   cout << "Error: " << e.what() << "\n";
   return 1;
}
return 0;
alexey-malov commented 9 years ago
    double matrixDet = Determinant(matrix);
    if (matrixDet < DBL_EPSILON)

проверять следует абсолютное значение. Отрицательное значение детерминанта - нормальное состояние

alexey-malov commented 9 years ago

Матрицы следует передавать по константной ссылке, а не просто по константному значению. В этом случае матрица при входе в функцию копироваться не будет

alexey-malov commented 9 years ago

Это править не обязательно:

bool InvertMatrix(Matrix3x3 const matrix, Matrix3x3 &invertedMatrix)

При помощи boost (http://boost.org) можно использовать опциональное значение

#include <boost/optional.hpp>

boost::optional<Matrix3x3> InvertMatrix(Matrix3x3 const& matrix)
{
    if (/*determinant is close to zero*/)
    {
        return boost::none;
    }
    else
    {
      return MultMatriByScalar(GetMinorMatrix(matrix), 1.0 / determinant);
    }
}

...
int main()
{
    auto invertedMatrix = InvertMatrix(matrix);
    if (invertedMatrix) // либо invertedMatrix.is_initialized(); вроде
    {
         PrintMatrix(*invertedMatrix); // либо PrintMatrix(invertedMatrix.get());
    }
}
alexey-malov commented 9 years ago

100*0,75 = 75