garbage values populating and printing elements from for-each loop

[technologist]

My goal is to both populate and print the my_array with for-each functions only. The program compiles fine but puts out junk numbers. How do make this work? If I compile using a more standard matrix, the values are fine.

#include<iostream>
#include<array>

using namespace std;

struct objects
{
	int x=0;
	int y=0;
};

void fill_print_array(array<objects,10> &my_array)
{

		int ctr = 1;
		for( objects elem:my_array)
		{
			elem.x = ctr*4;
			elem.y = ctr*3;
			ctr++;
		}
		
		//test fill values
		for(objects elem:my_array)
		{
			cout<<elem.x<<"\t"<<elem.y;
		}

}


int main()
{
	array<objects, 10>my_array;
	fill_print_array(my_array);

	return 0;
}

working copy using for()

#include<iostream>
#include<array>

using namespace std;

struct objects
{
	int a=0;
	int b=0;
};

void fill_and_print_array(array<objects,10> &int_array)
{		int  ctr(0);
		for(int x = 0; x<int_array.size(); ++x)
		{
			++ctr;
			int_array[x].a = ctr * 3.14159;	//magic nums testing only
			int_array[x].b = ctr*2;
		}


		//test fill values
		for(objects elem:int_array)
		{
			cout<<elem.a<<"\t"<<elem.b<<endl;
		}

}


int main()
{
	array<objects, 10>int_array;
	fill_and_print_array(int_array);

	return 0;
}

[shadowmouse]

Line 16 should be declaring elem as an objects&, so that operations on elem act on the actual element of my_array, rather than on a copy of it. Also you forgot the endl on line 26.

[cire]

Line 16 in the first snippet should be

for( objects& elem:my_array)

otherwise you're just modifying a copy of the element in the loop body.

[technologist]

made the corrections and working fine, thx!

operations on elem act on the actual element of my_array, rather than on a copy of it.

Why does this code only work with the ampersand (original storage addresses) -- there are plenty of other situations in which copying (-ampersand) although not ideal, still function.

[TheIdeasMan]

Why does this code only work with the ampersand (original storage addresses) -- there are plenty of other situations in which copying (-ampersand) although not ideal, still function.

It's because you are dealing with a copy of the array, and you want to assign values to the original array. Other functions which don't do assignment work well, although as you say inefficiently. Whenever you are dealing with a class of any sort (either STL or one of your own) prefer to pass by reference, and const if possible.

With your function, ( I know it's an exercise ) it should only do one conceptual thing. So have two functions: one to fill; the other to print. This is obviously more flexible. Not sure whether you knew this already :+)

In a print function as an example the reference in the range based for loop can be const as well as being a reference:

for( const objects& elem:my_array) {

With C++14, one can use auto as well - the compiler deduces the type:

for( const auto& elem: my_array) {

[technologist]

otherwise you're just modifying a copy of the element in the loop body.

True but even this code (value vs ref) won't produce garbage values that the above code gives if there is no reference symbol.

#include<iostream>
#include<array>

using namespace std;

int add_it_copy(int a, int b)
{
	int total(0);
	a = a + b;
	b = b +b;
	total = a + b;
	return total;
}

int add_it_ref(int &x, int &y)
{
	int total(0);
	x = x + y;
	y = y + y;
	total = x + y;
	return total;
}

int main()
{
int a(2),b(3),x(4), y(5);
int total_1(0), total_2(0);
	cout<<"pass by value"<<endl;
	cout<<add_it_copy(a,b)<<endl;
	cout<<add_it_copy(a,b)<<endl;
	cout<<add_it_copy(a,b)<<endl;
	cout<<"pass by reference :"<<endl;
	cout<<add_it_ref(a,b)<<endl;
	cout<<add_it_ref(a,b)<<endl;
	cout<<add_it_ref(a,b)<<endl;

	return 0;
}

[technologist]

Thx IDM

for( const auto& elem: my_array)

that's pretty cool

had I used that earlier I wouldn't have wasted time debugging

[TheIdeasMan]

had I used that earlier I wouldn't have wasted time debugging

Only if the values weren't being modified: obviously you couldn't use const if they are being changed. Another good reason to have function that does one thing only.

True but even this code (value vs ref) won't produce garbage values that the above code gives if there is no reference symbol.

The add_it_copy doesn't change the values of a and b in main, and it returns a value. The calls to add_it_copy all produce the same answer, but the calls to add_it_ref produce different answers because a and b are changed by the function.

Btw, it could be a little confusing that you used x and y as parameters, because it is a and b that are being changed in main() This is borne out by the compile warnings:

 In function 'int main()':
26:15: warning: unused variable 'x' [-Wunused-variable] 
26:21: warning: unused variable 'y' [-Wunused-variable] 
27:5: warning: unused variable 'total_1' [-Wunused-variable] 
27:17: warning: unused variable 'total_2' [-Wunused-variable]

I generally like to name the parameters the same as the arguments: it reinforces the idea that values are the same. Although they are only truly the same if passed by reference.

Also, with your function, delay the declaration of a variable until you have a sensible value to assign to it:

int add_it_ref(int &a, int &b)
{
	int total(0);
	a += b;
	b += b;
	int total = a + b;
	return total;
}

Can I persuade you to not have using namespace std; ?

[cire]

cire wrote:

otherwise you're just modifying a copy of the element in the loop body.

technologist wrote:

True but even this code (value vs ref) won't produce garbage values that the above code gives if there is no reference symbol.

The code you supplied to make your point is not anywhere close to the functionality of the code that makes your point moot.

This is more analogous to the original code:

#include<iostream>

using namespace std;

void suppress_uninitialized_error(int&) {}

void add_it_copy(int result, int a, int b)
{
    result = a + b;
}

void add_it_ref(int& result, int x, int y)
{
    result = x + y;

}

int main()
{
    int total;
    suppress_uninitialized_error(total);

    add_it_copy(total, 2, 3);
    cout << total << '\n';

    add_it_ref(total, 2, 3);
    cout << total << '\n';
}

[technologist]

Can I persuade you to not have using namespace std; ?

It will take a while to break that habit, but I'll go ahead and give it a try.