1

I am trying to write an OLS regression in C++. I want the user to specify how many observations and variables goes into the X (independent variable matrix). I want to write this as a function outside the int main() function. But everytime I return the array, I get a type error. How do I specify that I want function input_x to return a matrix/array?

 /******************************************************************************   
                                  Online C++ Compiler.    
    *******************************************************************************/
    
    #include <iostream>
    
    using namespace std;
    
    
    int input_x(int num_rows, int num_cols){
        
        
       int mat[num_rows][num_cols];
       int user_input;
           
        for (int i=0; i<num_rows; i++){
            for (int j=0; j<num_cols; j++){
                    cout<<"Enter the value X:";
                    cin>>user_input;
                    mat[i][j]=user_input;
                
            }
        }
    
    
       return mat;
    }
    
    
    
    int main()
    {
        int mat[1000][1000];
        int input;
        int rows;
        int cols;
        
        cout<<"How many row?";
        cin>> rows;
        cout<<"How many columns?";
        cin>> cols;
        

        //Enter the X data
        input_x(rows,cols);
        
        
    
    
        return 0;
    }
Fred Larson
  • 60,987
  • 18
  • 112
  • 174
jessica
  • 1,325
  • 2
  • 21
  • 35
  • 3
    The type of `mat` is `int**`. But you can't return a function local c-array like that. You need to use something that has proper copy semantics, like a `std::vector>`. – super Jan 06 '22 at 14:44
  • ok so my issue is with int** is it's a pointer variable (correct me if I am wrong). I cannot use functions such as sizeof(mat) to determine the size of the matrix. I lose a lot of attribute information by declaring it on the heap. – jessica Jan 06 '22 at 14:46
  • 1
    `input_x` is not declared to return a 2D integer array, it's declared to return an integer. And you can't actually return a native array, per se. See [Return array in a function](https://stackoverflow.com/q/3473438/10077). – Fred Larson Jan 06 '22 at 14:47
  • 1
    `int man[1000][1000]` -- That's C, not C++, and using this C construct in C++ is really bad practice. Check out [`std::vector`](https://en.cppreference.com/w/cpp/container/vector). Or [Boost.uBLAS](https://www.boost.org/doc/libs/release/libs/numeric/ublas/) if you want to go into matrices hardcore. But I guess before you go there, you need to understand basic data structures and function calls... – DevSolar Jan 06 '22 at 14:47
  • How about if I declare the memory size of `int mat[num_rows][num_cols]` by making it `int mat[10000][10000]`. Would that work? Now the memory of the array is declare before hand and memory is set aside for it. – jessica Jan 06 '22 at 14:50
  • 1
    @jessica You still can't return a local c-style array from a function. You need `std::vector>`. – super Jan 06 '22 at 14:52
  • 1
    @jessica: No. You'll just run out of stack space. Seriously -- `std::vector` or `boost::numeric::ublas::matrix` (perhaps `symetric_matrix`?) are the types you are looking for. – DevSolar Jan 06 '22 at 14:52
  • I did not know `mat[10000][10000]` is a C style array. All the teaching tutorials for C++ teach that as the way of creating an array. Ok thanks for letting me know! – jessica Jan 06 '22 at 14:55
  • btw why does `mat[10000][10000]` run out of stack space? I noticed that I had to reduce the size of the matrix just so I can get it to run. 10,000x10,000 array size is not that much! No? – jessica Jan 06 '22 at 14:57
  • 3
    @jessica If 381 GB is not A LOT to allocate on the stack, I don't know what is. :-) – super Jan 06 '22 at 15:03
  • 2
    @super Replying to your very first comment. The type of `mat` is `int [num_rows][num_cols]` and not `int**` which you incorrectly claimed. – Jason Jan 06 '22 at 15:17
  • 1
    "All the teaching tutorials for C++ teach that as the way of creating an array." -- There is a very unfortunate tendency for C++ course material to start on the C level. Whether that is because that's how the tutor learned it or whether the tutor thinks it will make students better appreciate "the C++ way" once they actually show it, I don't know. What I *do* know is that it is a very frustrating and error-prone way to learn C++. And also a good test for course material: If it starts with C arrays, naked pointers, and `new`, find something better. Because neither has a place in modern C++. – DevSolar Jan 06 '22 at 16:15
  • @AnoopRana Sure. Is that a relevant distinction in the comments here though? OP is looking to return a 2d-array of unknown dimension. That would be a `int**`. Now, getting all long a technical in comments are not usually a good idea and trying to explain the concepts of arrays decaying to pointer, VLAs and all the other details that could be relevant here would probably just confuse the OP more then it would help them. – super Jan 06 '22 at 17:46

4 Answers4

2

Mistake 1

In C++ the size of an array must be a compile time constant. So take for example,

int n = 10;
int arr[n]; //INCORRECT because n is not a compile time constant

The correct way to write the above would be

const int n = 10;
int arr[n]; //CORRECT

Similarly in your code:

int mat[num_rows][num_cols];//INCORRECT because num_rows and num_cols are not constant expressions

Mistake 2

The return type of your function input_x is int.

But the type of mat inside the function input_x is int [num_rows][num_cols] which decays to int (*)[num_cols] due to type decay.

So essentially the return type of your function(int) and what you're actually returning(int (*)[num_cols]) doesn't match and hence you get the following error:

error: invalid conversion from ‘int (*)[num_cols]’ to ‘int’

Solution

To solve these problems you should use std::vector instead of built in array as shown below:

#include <iostream>
#include <vector> 
//this function takes vector by reference and returns nothing 
void input_x(std::vector<std::vector<int>> &vec){
    
    for(auto &rows: vec)
    {
        for(auto &cols: rows)
        {
            std::cout << "Enter element: "<<std::endl;
            std::cin >> cols; 
        }
    }


   //NO NEED TO RETURN ANYTHING SINCE THE VECTOR IS PASSED BY REFERENCE SO THE CHANGE IS ALREADY REFLECTED ON THAT PASSED VECTOR
}

int main()
{
    int rows;
    int cols;
    
    std::cout<<"How many row?"<<std::endl;
    std::cin>> rows;
    std::cout<<"How many columns?"<<std::endl;
    std::cin>> cols;
    
    //create 2D vector 
    std::vector<std::vector<int>> myVec(rows, std::vector<int>(cols));
    

    //call the function and pass the vector as argument 
    input_x(myVec);

    return 0;
}
Jason
  • 36,170
  • 5
  • 26
  • 60
1

As the comment states, your return type of the function input_x is wrong.

Your matrix mat is an array of arrays of int. This is a c-style array and you can only return it as an int** which would directly lead to a dangling reference.

Instead of using c-style arrays it's better to use standard containers, like std::vector as Fred Larson pointed out.

See the documentation here about std::vector In general https://en.cppreference.com is a good start to see if any standard library type suits your needs.

You can initialize your matrix like this:

std::vector<std::vector<int>> mat(num_rows)
for (auto & row : mat)
    row.resize(num_cols);

Don't forget to include the respective header

#include <vector>
  • `std::vector> mat(num_rows, std::vector(num_cols));` would be the more idiomatic approach. – super Jan 06 '22 at 14:58
  • @BKN So I should avoid providing a helpful comment because the information is available on other questions/answers on SO? – super Jan 06 '22 at 15:10
  • @super: my comment was not for you. Your comment is indeed helpful. I meant the answer by Svenja ends up duplicating...I should have used *answer* instead of *comment* to avoid confusion. – BZKN Jan 06 '22 at 15:15
  • @Svenja I think your answer just ends up duplicating the already provided comments for a question that is almost a duplicate question in SO :) – BZKN Jan 06 '22 at 15:18
  • @BKN Alright :) I'm new to contributing to SO. When I wrote the answer there was only the comment from Fred Larson and I thought since the person asking might not be experienced with c++ types I elaborate. Should I delete the answer? – Svenja Mehringer Jan 06 '22 at 15:40
1

One of the strengths of C++ is that many complex data types already exist in the standard library. And even more exist in common extension libraries like Boost. You would be well-advised to use those instead of "making do" with C style arrays or structs, if for no other reason than to avoid error-prone manual memory management.

Svenja showcased the "vector of vector" approach using the standard type std::vector. But there is an even better match for your use case in Boost.uBLAS' Matrix type:

#include <iostream>
#include <boost/numeric/ublas/matrix.hpp>
#include <boost/numeric/ublas/io.hpp>

// Avoid "using namespace". Keep the global namespace clean,
// and use explicit namespacing for minimal ambiguity.
namespace bu = boost::numeric::ublas;

// Use (unsigned) size_t for sizes. Use padding spaces for
// readability.
bu::matrix< int > input_x( size_t num_rows, size_t num_cols )
{
    bu::matrix< int > matrix( num_rows, num_cols );

    // Use "speaking" variable names instead of "i" and "j".
    // Use prefix "++"; while it makes no difference with POD
    // types, it does make a difference with complex ones.
    for ( size_t r = 0; r < num_rows; ++r )
    {
        for ( int c = 0; c < num_cols; ++c )
        {
            // Give all the useful information you can.
            // Don't forget to std::endl your output if
            // the next thing you do is expecting input.
            std::cout << "value " << r << ", " << c << std::endl;

            // Avoid the use of "named temporary" variables.
            // Name only what you actually need later on, and
            // if you can (like here), just work without any
            // intermediary like "user_input".
            std::cin >> matrix( r, c );
        }
    }

    return matrix;
}

int main()
{
    size_t rows;
    size_t cols;

    std::cout << "How many rows?" << std::endl;
    std::cin >> rows;
    std::cout << "How many columns?" << std::endl;
    std::cin >> cols;

    // In C++ you do not need to declare all variables at the
    // beginning of the block. Declare at the point you actually
    // need the variable.
    bu::matrix< int > matrix = input_x( rows, cols );

    return 0;
}
DevSolar
  • 67,862
  • 21
  • 134
  • 209
-1

The value returned by the function is not stored.

input_x(rows,cols); // should be stored in a variable

Also, you have to change the return type of function to a matrix.

int input_x(int num_rows, int num_cols){

To solve the problem:

  1. Create a matrix dynamically
  2. Return the matrix

Here is the code for the above approach.

/******************************************************************************
                                  Online C++ Compiler.
    *******************************************************************************/

    #include <iostream>

    using namespace std;


    int** input_x(int num_rows, int num_cols){

       int **mat = (int **)malloc(num_rows * sizeof(int *));
       int user_input;

        for (int i = 0; i < num_rows; i++) {
            mat[i] = (int *)malloc(num_cols * sizeof(int));
        }
        for (int i=0; i<num_rows; i++){
            for (int j=0; j<num_cols; j++){
                    cout<<"Enter the value X:";
                    cin>>user_input;
                    mat[i][j]=user_input;

            }
        }

       return mat;
    }



    int main()
    {
        int** mat;
        int input;
        int rows;
        int cols;

        cout<<"How many row?";
        cin>> rows;
        cout<<"How many columns?";
        cin>> cols;


        //Enter the X data
        mat = input_x(rows,cols);
/*
        for(int i=0;i<rows;i++){
            for(int j=0;j<cols;j++){
                cout<<mat[i][j]<<" ";
            }
            cout << endl;
        }
*/



        return 0;
    }
csgeek
  • 711
  • 6
  • 15
  • 2
    "To solve the problem: 1. Create a matrix dynamically." -- No. Because now your program is leaking memory. Don't use C constructs in C++. – DevSolar Jan 07 '22 at 08:02
  • Yeah. This approach has memory leaks. I forgot to mention – csgeek Jan 07 '22 at 09:05