moving variable declarations outside for loop

Question

File: bodies.cpp

  for (int i=0; i<n; ++i) {
    phys_vector pos{xdist(re), ydist(re)};
    double mass = mdist(re);
    body b{pos.x, pos.y, mass};
    bodies.push_back(b);
  }

File: bodies.h

public:
  bodies_aos() = default;
private:  
  std::vector<phys_vector> compute_forces(const simulation_parameters & param);    
private:
  std::vector<body> bodies;
};

My intention is to define all the variables outside the loop. My approach (which afterwards I found out it was incorrect, as it does not return the same results) is the following one:

bodies.cpp ->Modified

  int i;
  double mass;
  vector<phys_vector> pos;
  std::vector<body> b;

  for (i=0; i<n; ++i) {
    phys_vector pos{xdist(re), ydist(re)};
    mass = mdist(re);
    body b{pos.x, pos.y, mass};
    bodies.push_back(b);
  }

Unfortunately, it did not return the same results due to a bad initialization of variables b and/or pos, but it does not raise any errors when compiling.

Does anyone knows how this could be solved in order to obtain the same results as in the first case?

Are you doing this in hope of some performance gains? If you are, you surely have done some serious profiling, right? — Pablo, Nov 20 '15 at 01:28
What are the differences in results? It looks like it should put the same values in `bodies`. What other results are you expecting? — Barmar, Nov 20 '15 at 01:32
@Pablo no, I did not do a profiling as this is part of an exercise intended to understand OpenMP (where I get constantly stacked in C++ part) where we should only include pragma sentences as the smallest C++ program changes possible, as the programs are intentionally bad programmed in order to increase computation time and therefore the reduction of time with OpenMP is greater and easier to see. — Alvaro Gomez, Nov 20 '15 at 01:42

Elazar · Accepted Answer · 2015-11-20T01:33:01.867

3

You did not move all the variables. pos and b are still defined inside the loop, hiding the outside definitions (and the outer pos became a vector<phys_vector> for some reason). The outer variables are untouched inside the loop, hence are not initialized the way you intended them to.
You should not do this without good reason. The smaller the scope of a variable, the better you will be able to reason about the code.

My best guess is that you want something like this

vector<phys_vector> pos;

for (int i=0; i<n; ++i) {
    pos.emplace_back(xdist(re), ydist(re));
    double mass = mdist(re);
    bodies.emplace_back(pos.back().x, pos.back().y, mass);
}
// use pos and bodies

edited Nov 20 '15 at 01:33

answered Nov 20 '15 at 01:21

Elazar

20,415
4
46
67

This doesn't really explain why it's not producing the same results. Nothing uses the `pos` and `b` variables outside the loop, so it shouldn't matter if they're uninitialized. – Barmar Nov 20 '15 at 01:31
@Barmar from "Unfortunately, it did not return the same results due to a bad initialization of variables b and/or pos" I understand that there is some code outside this loop, using those variables. – Elazar Nov 20 '15 at 01:34
1

But what would the original code have done that this is "not the same" as? – Barmar Nov 20 '15 at 01:36
That's a good question. I believe that the OP could elaborate. – Elazar Nov 20 '15 at 01:39
@Barmar yes, this is part of the code, the result is obtained in another part, where these results are used. – Alvaro Gomez Nov 20 '15 at 01:44
1

@AlvaroGomez Please add to the question, showing what you were doing in the original code, and what you're trying to do in the new code, and how the results are different. As it is, it's really unclear what the problem is. – Barmar Nov 20 '15 at 01:46

moving variable declarations outside for loop

1 Answers1