0

Not sure why the for loop won't save the correct values in the 2D array when I printed to check. Any ideas?

#include <iostream>
using namespace std;

int row, col; 

int main()
{
int num;
int val[row][col];

cout << "How many rows are there?" << endl;    
cin >> row;
cout << "How many columns are there?" << endl;
cin >> col;
cout << "Enter values for the matrix: " << endl;

for (int i = 0; i < row; i++)                 
{
    for (int j = 0; j < col; j++)
    {
        cin >> val[i][j]; 
    }   
}
return 0;
}
Lucy
  • 11
  • 1
  • 1
  • 2
  • 1
    `int val[row][col];` This is not valid C++ – DimChtz Apr 25 '18 at 17:51
  • and you haven't even initialized `row` or `col` at that point -> *undefined behavior* (on top of using a compiler extension) – UnholySheep Apr 25 '18 at 17:52
  • @UnholySheep `row` and `col` get [zero-initialized](http://en.cppreference.com/w/cpp/language/zero_initialization) since they have static storage duration. – Pezo Apr 25 '18 at 18:15
  • Possible duplicate of [How do I declare a 2d array in C++ using new?](https://stackoverflow.com/questions/936687/how-do-i-declare-a-2d-array-in-c-using-new) – user8145466 Apr 25 '18 at 18:32

4 Answers4

3
#include <iostream>
using namespace std;

int main()
{
    int row, col; 

    cout << "How many rows are there?" << endl;    
    cin >> row;
    cout << "How many columns are there?" << endl;
    cin >> col;
    cout << "Enter values for the matrix: " << endl;

    // check if row and col > 0
    int* val = new int[row * col];

    for (int i = 0; i < row; i++)                 
    {
        for (int j = 0; j < col; j++)
        {
            cin >> val[i * col + j]; 
        }   
    }
    delete[] val;
    return 0;
}
Ali Asadpoor
  • 327
  • 1
  • 13
1

This doesn't do what you think it does. First of all, row and col are zero-initialized at program startup. Then you have int val[row][col]; which isn't valid C++ but rather a C variable length array. Since at this point row and col are both 0, this array will be of length zero.

In your loop you then read a bunch of values, overwriting what is on the stack and leading to undefined behavior.

You should instead use something that is dynamically allocated, like std::vector or a proper matrix class from the math library of your choosing. Using dynamic allocation manually (new int[row * col] as suggested by Ali) is generally not recommended, since it's very easy to get memory leaks this way, especially if exceptions are involved.

Pezo
  • 1,458
  • 9
  • 15
0

If variable length arrays would be supported in C++ you could write what you wanted by just shifting:

int val[row][col];

To the point where row and col are known. Check this post: Why aren't variable-length arrays part of the C++ standard? Otherwise your code has undefined behavior. You should use dynamic allocation.

Mateusz Wojtczak
  • 1,621
  • 1
  • 12
  • 28
0

First, you can't have

int val[row][col];

before row and col have known values. Second, that kind of 2D array initialization is only standard in C.

You would need to manually allocate the array on the heap using the C++ operator new[] (similar to the malloc function in C). However that's not idiomatic C++ and the whole point of the language is to avoid doing that which is why I won't explain how to do it.

The proper C++ way to accomplish what you want is to use an std::vector, which a very powerful wrapper around C style arrays that automatically allocates and de-allocates memory for you (and a lot of other things).

Here's the simplest way to do that:

#include <iostream>
#include <vector>

using namespace std;

int main()
{
    int row;
    cout << "How many rows are there?" << endl;
    cin >> row;

    int col;
    cout << "How many columns are there?" << endl;
    cin >> col;

    int num;
    cout << "Enter values for the matrix: " << endl;
    cin >> num;

    vector<vector<int>> values(col); //initialize outer vector with col inner vectors

    for (int i = 0; i < row; i++)
    {
        for (int j = 0; j < col; j++)
        {
            values[i].push_back(num);
        }
    }

    for (int i = 0; i < row; i++)
    {
        for (int j = 0; j < col; j++)
        {
            cout << values[i][j];
        }

        cout << endl;
    }

    return 0;
}

Also, I advise you name your variables with more meaning and that you avoid using namespace std.

EDIT: From the way you approach the program, I assume in my answer you are familiar with C. If that's not the case, and you're following a book or tutorial, you should find a better book or tutorial.

  • It's for my c++ class but the book uses C / C++ . We haven't been using other complex ways to approach problems. – Lucy Apr 25 '18 at 18:26
  • Using a vector is not a complex way, it's simply the proper way. C / C++ is not a thing as the languages are completely different in practice, despite ostensibly being closely related. If you simply must use plain arrays due to the constraints of the class itself, see [this question](https://stackoverflow.com/questions/936687/how-do-i-declare-a-2d-array-in-c-using-new). However, keep in mind that despite working completely fine, that approach is basically always wrong in C++ unless you really know what you're doing and have a very good reason for doing so. – user8145466 Apr 25 '18 at 18:31
  • I'm not sure why the lesson hasn't included that yet. But on my original code, i moved "int val [row][col];" to before the for loop and it works now. It may not be valid C++ but the professor is using it lol. – Lucy Apr 25 '18 at 18:40