CODEWITHSUNDEEP
c++c++11vector
mm25
mm25

Reputation: 11

issue with different methods to loop over a std::vector and getting pointer to its elements

let me preface by saying that i'm not a c++ expert here, so i apologize in advance if what i'm trying to figure out is trivial for most. i have tried to find the solution to this in other posts, but this time i haven't succeeded.

at the end of the post you can find the standalone code that reproduces the 2 issues i'm about to detail [*].

i'm using a class A containing a std::vector<float> as member, which can be accessed via a const method.

loop #1

  for(const auto& af : a.vec()){

    std::cout << af << " (address=" << &af << ")\n";
  }

loop #2

  for(unsigned int i=0; i<a.vec().size(); ++i){

    const auto& af = a.vec().at(i);
    std::cout << af << " (address=" << &af << ")\n";
  }

the 2 approaches seem to yield 2 different results. loop #1 works as expected. when using loop #2, i get that the value of some of the vector elements is now (printed out as) zero, but the memory addresses are still the right ones. naively, i expected the two approaches to give the same output. what is the reason why they differ?

below i wrote down some instructions to reproduce these two issues.

any insight on the reason why these 2 issues arise (and/or general suggestions on the implementation below) is welcome.

thanks

[*] i'm compiling the code below (test.cc) with g++ version 4.9.1 in SLC6, doing g++ test.cc -o test -std=c++0x.

#include <iostream>
#include <vector>
#include <cmath>
#include <limits>

class A {
 public:
  explicit A() {}
  virtual ~A() {}

  std::vector<float> a_vec() const { return a_vec_; }

  void add_obj(const float f){

    a_vec_.push_back(f);

    return;
  }

 private:
  std::vector<float> a_vec_;
};

template<typename T>
const T * closest0(const float& f, const std::vector<T>& fvec){

  const T* next=0;

  float deltarmin = std::numeric_limits<float>::infinity();
  for(unsigned int i=0; i<fvec.size(); ++i) {
    const T & fi = fvec[i];
    const float dr = fabs(fi-f);
    if(dr < deltarmin){

      deltarmin = dr;
      next = &fi;
    }
  }

  return next;
}

int main(int argc, char** argv){

  if(argc-1 != 1) return 0;

  const float ref(atoi(argv[1]));
  ////

  A a;
  a.add_obj(25.);
  a.add_obj(35.);
  a.add_obj(45.);
  a.add_obj(55.);

  std::cout << "\n*** input vector ***********\n";
  for(unsigned int i=0; i<a.a_vec().size(); ++i){
    std::cout << "a.a_vec().at(" << i << ")=" << a.a_vec().at(i) << " (address=" << &a.a_vec().at(i) << ")\n";
  }
  std::cout <<   "****************************\n";

  int tryN(0);
  ////

  ++tryN;
  std::cout << "\n--- loop:(const auto&)" << std::endl;
  for(const auto& af : a.a_vec()){

    std::cout << "#" << tryN << " " << af << " (address=" << &af << ")\n";
  }
  std::cout << "---------------" << std::endl;
  ////

  ++tryN;
  std::cout << "\n--- loop:(index) + 'const auto& ... at()'\n";
  for(unsigned int i=0; i<a.a_vec().size(); ++i){

    const auto& af = a.a_vec().at(i);
    std::cout << "#" << tryN << " " << af << " (address=" << &af << ")\n";
  }
  std::cout << "---------------" << std::endl;
  ////

  std::cout << "\n---------------\n\n";

  const float* fclo0 = closest0(ref, a.a_vec());
  std::cout << " *** f-closest0(" << ref << ")=" << *fclo0 << " (address=" << fclo0 << ")\n";

  return 0;
}

an example of the output i'm getting is

$ ./test 33


*** input vector ***********

a.a_vec().at(0)=25 (address=0x1a4f030)

a.a_vec().at(1)=35 (address=0x1a4f034)

a.a_vec().at(2)=45 (address=0x1a4f038)

a.a_vec().at(3)=55 (address=0x1a4f03c)

****************************



--- loop:(const auto&)

#1 25 (address=0x1a4f030)

#1 35 (address=0x1a4f034)

#1 45 (address=0x1a4f038)

#1 55 (address=0x1a4f03c)

---------------


--- loop:(index) + 'const auto& ... at()'

#2 6.05888e-38 (address=0x1a4f030)

#2 0 (address=0x1a4f034)

#2 45 (address=0x1a4f038)

#2 55 (address=0x1a4f03c)

---------------


---------------


 *** f-closest0(33)=0 (address=0x1a4f034)

Upvotes: 0

Views: 92

Answers (5)

sfjac
sfjac

Reputation: 7294

@juanchopanza correctly points out that a_vec() is returning by value, but that doesn't explain the output completely. The issue is that 

 const auto& af = a.a_vec().at(i);
 std::cout << "#" << tryN << " " << af << " (address=" << &af << ")\n";

The first line binds a reference to a value that was returned from a temporary copy of the data, which then goes out of scope, so by the time you access af again, the underlying memory has been reclaimed and may have been overwritten.

The reason the const reference doesn't extend the lifetime of the temporary is that it is a reference to a result of using the temporary in the expression, not a reference to the temporary itself. If you instead did

  const auto &vec = a.a_vec();
  std::cout << "#" << tryN << " " << vec.at(i) << " (address=" << &vec.at(i) << ")\n";

then it should be OK. (Though the right answer is to have a_vec return a const ref to the vector in the first place.)

Upvotes: 2

Mercyful
Mercyful

Reputation: 107

for(unsigned int i=0; i<a.a_vec().size(); ++i){

    const &auto af = a.a_vec().at(i);
    std::cout << "#" << tryN << " " << af << " (address=" << &af << ")\n";
}

for #2, Remove the & before auto and it will work the same. Your getting a Value not a reference in the second loop!

const auto af = a.a_vec().at(i);

Upvotes: 0

Anon Mail
Anon Mail

Reputation: 4770

I think this statement is suspect:

const auto& af = a.a_vec().at(i);

I believe the std::vector returned by a.a_vec() goes out of scope after the statement ends. Yet you still have a reference to one of it's members.

You can fix this by taking a reference to the entire vector like this:

const auto & av = a.a_vec();

Or copying an element like this:

const auto af = a.a_vec().at(i);

Upvotes: 0

Kamal Sadek
Kamal Sadek

Reputation: 86

The problem is in the second loop

for(unsigned int i=0; i<a.a_vec().size(); ++i){ const auto& af = a.a_vec().at(i); std::cout << "#" << tryN << " " << af << " (address=" << &af << ")\n"; }

It's because of the declaration of the variable, const auto&. If you were to remove the ampersand and hold it by value instead of by reference, it works, so just declare it as const auto

Upvotes: 0

juanchopanza
juanchopanza

Reputation: 227468

A::a_vec() returns by value. You get a new vector each time you call it. You could fix this by returning a reference

const std::vector<float>& a_vec() const { return a_vec_; }

or making a_vec_ public and accessing it directly.

Upvotes: 2

Related Questions