Iterator on a map never atteins map.end()

Question

why the iteration in the code below doesn't work ? I mean the iterator is incremented but evaluating this expression o!=RateCurve.end() always give true.

i need such a function because i'm using it in a wrapper of a map to contruct an interest rate curve.

#include <iostream>     
#include <algorithm>    
#include <math.h>       
#include <string>       
#include <map>          
#include <exception>    
#include <vector>       
#include <stdio.h>      

using namespace std;


map<double, double>::iterator getIt(map<double, double> o){
    return o.begin();
}

int main ()
{
    map<double, double> RateCurve;

    RateCurve[3.3  ]=0.034 ;
    RateCurve[1.2  ]=0.03  ;
    RateCurve[0.2  ]=.001  ;
    RateCurve[6.1  ]=.023  ;



    map<double, double>::iterator o=getIt(RateCurve);
    while (o!=RateCurve.end()){

        cout << "RateCurve[" << o->first << "] = " << o->second << endl;
        ++o;
    }

}

it's unrelated to your question, but I can't see why you need to define `getIt` when you can directly call `begin()` - your function does nothing more than the latter. — Marinos K, Mar 08 '16 at 16:45
`for ( auto const& o : RateCurve ) { ... }` is much cleaner. — zdf, Mar 08 '16 at 17:29

score 3 · Answer 1 · edited May 23 '17 at 12:15

3

getIt(map<double, double> o)

You copy the map, so the iterator returned points into an entirely unrelated map to the one you wanted. Worse, the copy gets destroyed at the end of the function call, and the iterator you try to use is no longer valid, so you have Undefined Behaviour.

Make getIt take a reference, and since you don't actually change the elements, you can make it const too. Then you need to change the return type:

std::map<double, double>::const_iterator
getIt(map<double, double> const& o)
{
    return o.begin();
}

Also, please reconsider your use of bad practices using namespace std; and endl.

edited May 23 '17 at 12:15

Community

1
1

answered Mar 08 '16 at 16:36

BoBTFish

19,167
3
49
76

When the data is only used for reading I would even suggest to pass a const reference and return `map::const_iterator`. – Simon Kraemer Mar 08 '16 at 16:39
so instead of using namespace, should I use std:: each time? – Dave ddd Mar 08 '16 at 16:43
1

@Daveddd That is my preference, and one shared by most experienced C++ programmers I know. It makes the code a lot easier to read, even if it makes it slightly slower to type (and I have a vim mapping to enter `std::` with one keystroke). – BoBTFish Mar 08 '16 at 16:45
1

@Daveddd You can explicitly use names out of a namespace. E.g. `using std::cout; using std::endl;` will still let you write `cout << "Hello" << endl;`, but you won't accidentially have ambigious names. – Simon Kraemer Mar 08 '16 at 16:45

Iterator on a map never atteins map.end()

1 Answers1