-1

I need to populate a 2d array (that represents a matrix) in a function in c++.

The function looks like this:

double** makeMatrix (const char c, double a) {
    double matrix[3][3];
    
    switch (c) {
        case 'x' :
            matrix = {{1, 0, 0), {0, cos(a), -sin(a)}, {0, sin(a), cos(a)}};
            break;
        case 'y' :
            matrix = {{cos(a), 0, sin(a)}, {0, 1, 0}, {-sin(a), 0, cos(a)}};
            break;
        case 'z' :
            matrix = {{cos(a), -sin(a), 0}, {sin(a), cos(a)}, {0, 0, 1}};
            break;
    }
    return matrix;
}

Needless to say, this function doesn't work.

What am I doing wrong here?

I clearly have an issue due to the mismatch of the variable type I should return vs the one I'm returning.

I am... Lost in the clouds.

  • 1
    Aside from the fact you're retuning a dangling pointer (oops), don't attempt to guess the C++ syntax. Assign the elements one by one. By the way your `z` case looks funny from a mathematical perspective. Actually are there other mistakes? Looks like you've slipped a couple of signs. Is the determinant 1? You check - I'm too old. – Bathsheba Dec 03 '20 at 17:29
  • 1
    Even the fact that you are returning a pointer to a stack object indicates you are lost indeed. Forget all of that, get a good book and use std::vector. – Michael Chourdakis Dec 03 '20 at 17:29
  • 1
    Here's the C dupe (https://stackoverflow.com/q/8886375/2602718). The C++ answer is probably "Use a vector." – scohe001 Dec 03 '20 at 17:31
  • 1
    To my mind the answer is to use a matrix class from a linear algebra library: I use BLAS which is part of the Boost distribution. – Bathsheba Dec 03 '20 at 17:32
  • 1
    This looks like a rotation matrix. If that's the case you're going to be doing matrix math and you should use a matrix class from a linear algebra library to simplify your life. – Pranav Hosangadi Dec 03 '20 at 17:33
  • 1
    @PranavHosangadi: Indeed, except that it's *almost* a rotation matrix! – Bathsheba Dec 03 '20 at 17:47
  • No need to mock. I didn't copy&paste this function, I wrote it anew here. Thank you anyway. – Jimmy Scionti Dec 03 '20 at 18:12

2 Answers2

1

There is an urgent version for you.

#include <iostream>
#include <cmath>
#include <algorithm>
using namespace std;
void makeMatrix (const char c, const double a, double *mtx) {
    double matrix[3][3];
    for (int i=0; i<3; i++)
        for (int j=0; j<3; j++) matrix[i][j] = (i == j)? 1.0 : 0.0;
    switch (c) {
        case 'x' :
            matrix[1][1] = cos(a);
            matrix[1][2] = -sin(a);
            matrix[2][1] = sin(a);
            matrix[2][2] = cos(a);
            break;
        case 'y' :
            matrix[0][0] = cos(a);
            matrix[0][1] = -sin(a);
            matrix[2][0] = sin(a);
            matrix[2][2] = cos(a);
            break;
        case 'z' :
            matrix[0][0] = cos(a);
            matrix[0][1] = -sin(a);
            matrix[1][0] = sin(a);
            matrix[1][1] = cos(a);
            break;
    }
    std::copy_n(&matrix[0][0], 9, mtx);
    return;
}
int main()
{
    double mtx[3][3];
    makeMatrix('x', M_PI/3.0, &mtx[0][0]);
    for (int i=0; i<3; i++)
    {
        for (int j=0; j<3; j++) std::cout << mtx[i][j] << ",  ";
        std::cout << std::endl;
    }
}
ytlu
  • 412
  • 4
  • 9
  • `std::copy_n(&matrix[0][0], 9, mtx);` looks like UB although it'll _probably_ work. – Ted Lyngmo Dec 03 '20 at 18:25
  • Thank you, but I was actually for something more "clever" in the sense that I wanted to avoid typing several "matrix[i][j]". I'm also interested in learning the general "how-to", not just how to fix this. There must be some way to populate a matrix after the initialization, in one line, isn't it? But, nontheless, thank you for this. :) – Jimmy Scionti Dec 03 '20 at 18:27
  • @TedLyngmo What is UB? – ytlu Dec 03 '20 at 18:37
  • 1
    @JimmyScionti I see. I had a matrix class, but that is too big for learning purpose. I may make a small one for you. Using initailization list for matrix is definitely not a simple job. You might put it away for now. – ytlu Dec 03 '20 at 18:40
  • @ytlu [Undefined behavior](https://en.cppreference.com/w/cpp/language/ub). It'll happen when it writes at `&matrix[0][3]` which is out of bounds - but as I said, it'll probably work anyway. I don't know of any implementation where that wouldn't be the same as `&matrix[1][0]` but compilers are allowed to assume that you don't write out of bounds though, so who knows what could happen. – Ted Lyngmo Dec 03 '20 at 18:40
  • @TedLyngmo Get UB,. Thank you. The std::copy treated the matrix as 1-d array. Declara a[3][3] matrix, it is 1-d of 9 elements internally. – ytlu Dec 03 '20 at 18:45
  • @ytlu Yes, and still `matrix[0][3] = something;` makes it have undefined behavior. I'm pretty sure that if you replace it with two loops to make it _not_ UB, it'll optimize it to the same code that your `copy_n` results in (probably a `memcpy` or `memmove`). – Ted Lyngmo Dec 03 '20 at 18:47
  • @ytlu So, no population after initialization "out of the box" ️ Alright, I'll try with vectors of vectors. I'm usually luckier with strictly c++ classes/stuff. Maybe copying one variable into the other might work. I really don't want to code too much for this tiny script I need. Thank you, ytlu. Thank you for being gentle especially :) – Jimmy Scionti Dec 03 '20 at 18:50
  • @TedLyngmo There is no matrix[0][3]. It works as double *ptr = &matrix[0][0], then ptr[0], ptr[1], ptr[2], ....ptr[8]. What is concerned is the method of storage, e.g. c is row major, and fortran is column major. The different method renders different mapping between ptr[i] and the corresponding matrix[a][b]. – ytlu Dec 03 '20 at 18:52
  • 1
    @JimmyScionti There is brace-enclose initialization for matrix using std::initializer_list < std::initializer >. Youe have to write a matrix class with a constructor that takes this nested std::initializer_list as argument. It is sort of complicated for now. – ytlu Dec 03 '20 at 18:57
  • @TedLyngmo &matrix[0][0] is a double pointer, not a pointer for double[3]. Matrix[0] is a const double pointer with size 3, and so matrix[1], matrix[2]. BTW, how tp make a word in shaded background? I tried text , but don't work. – ytlu Dec 03 '20 at 19:07
  • @ytlu I'm fully aware how it's stored. `&matrix[0][3]` will be equal to `ptr + 3` and `&ptr[3]` (and even `&3[ptr]`). I'm just saying that it's UB and rewriting it to _not_ have UB would most probably result in the same assembly code. Probably. Compilers and optimization can sometimes make assumptions that you haven't made written out of bounds, which you do, since `matrix` isn't a `double[9]`. – Ted Lyngmo Dec 03 '20 at 19:09
  • One single backtick before and one after makes it gray. ... and there should be no spaces after the first backtick. – Ted Lyngmo Dec 03 '20 at 19:10
  • 1
    @TedLyngmo 'Thank you'. It is 'great'. – ytlu Dec 03 '20 at 19:15
0

This is a simple template matrix class with enbrace-enclosed constructor. Save it as tMatrix.h, and place it in your working directory together with the .cpp file.

#ifndef __matrix_class___
#define __matrix_class___
#include <iostream>
#include <initializer_list>
#include <iomanip>
#include <complex>
template <typename T> class tMatrix
{
 public:
     T *ptr;
     int col, row, size;
     inline T* begin() const {return ptr;}
     inline T* end() const {return this->ptr + this->size;}
     inline T operator()(const int i, const int j)const { return ptr[i*col+j]; }
     inline T&operator()(const int i, const int j) { return ptr[i*col+j]; }
     inline tMatrix(): col{0}, row{0}, size{0}, ptr{0} {;}
     tMatrix(const int i, const int j): col(j), row(i), size(i*j)
     {
         ptr = new T [this->size] ;
     }
     tMatrix(const std::initializer_list< std::initializer_list<T> > s):tMatrix<T>(s.size(), s.begin()->size())
     {
        int j = 0;
       for (const auto& i : s) {  std::copy (i.begin(), i.end(), ptr + j*col); ++j ; }
     }
     tMatrix(const tMatrix<T>&a) : tMatrix<T>(a.row, a.col)
     {
         std::copy(a.begin(), a.end(), this->ptr);
     }
     tMatrix& operator=(const tMatrix<T>&a)
     {
         std::copy(a.begin(), a.end(), this->ptr);
         return *this;
     }
    ~tMatrix() {delete [] this->ptr;}
 };
 template <typename X> std::ostream& operator<<(std::ostream&p, const tMatrix<X>&a)
{
    p << std::fixed;
    for (int i=0; i<a.row; i++) {
        for (int j=0; j <a.col; j++) p << std::setw(12) << a(i, j);
        p << std::endl;
    }
    return p;
}
using iMatrix = tMatrix<int>;
using rMatrix = tMatrix<double>;
using cMatrix = tMatrix<std::complex<double> >;
#endif

Then modify your program as:

#include <cmath>
#include "tMatrix.h"
using namespace std;
rMatrix makeMatrix (const char c, double a) {
   rMatrix mtx(3,3);
   double cs = cos(a), ss = sin(a);
   switch (c) {
       case 'x' :
          mtx = rMatrix( { {1, 0, 0}, {0, cs, -ss}, {0, ss, cs} } );
          break;
       case 'y' :
          mtx = rMatrix( { {cs, 0, ss}, {0, 1, 0}, {-ss, 0, cs} } );
          break;
       case 'z' :
          mtx = rMatrix( { {cs, -ss, 0}, {ss, cs, 0}, {0, 0, 1} } );
          break;
       default:
          break;
     }
   return mtx;
}
int main()
{
  char axis = 'x';
  double angle = M_PI / 180. * 60.;
  rMatrix mtx = {{0,0,0}, {0,0,0}, {0,0,0}}; //initial a 3x3 matrix with all 0s
  std::cout << mtx << std::endl;
  std::cout << makeMatrix(axis, angle) <<std::endl;
  return 0;
}
ytlu
  • 412
  • 4
  • 9