Iterating through map C++

Question

I have a map that is storing components of a polynomial. The key is the exponent and the value is the coefficient.The map that has integer keys and integer values. I am iterating in my print function, the program crashes when I do my iteration. As I was putting keys into my arrays everything seemed to check out. The input file has this format -1 0 5 1 20 3 -9 2 -2 1 1 2 -2 3 1 9,where every pair is (coefficient, exponent).`

//header
#ifndef poly_h
#define poly_h
#include <map>
class poly {

private:
    std::map<int, int>* pol;

public:
    poly(char* filename);
    poly& operator+(poly& rhs);
    poly& operator-(poly& rhs);
    poly& operator*(poly& rhs);
    void print();
    ~poly();
};
#endif

//cpp file
#include "poly.h"
#include <iostream>
#include <fstream>
using namespace std;
poly::poly(char* filename)
{

    map<int, int>* pol = new map<int, int>;
    ifstream input_file;
    input_file.open("input.txt");
    int coe;
    int exp;

    while (input_file >> coe) {
        input_file >> exp;
        cout << coe << " ^" << exp << " ";
        map<int, int>::iterator it = pol->find(exp);
        if (it == pol->end()) {
            (*pol)[exp] = coe;
        }
        else {
            (*pol)[exp] += coe;
            if ((*pol)[exp] == 0) {
                pol->erase(it);
            }
        }
        cout << (*pol)[exp];
        //print();
        cout << endl;
    }
    input_file.close();
}
void poly::print()
{
    cout << "inside print<<endl;                                                                   
        for (map<int, int>::iterator outer_iter = pol->begin(); outer_iter != pol->end(); ++outer_iter);
    cout << "done";
}
poly::~poly()
{
    delete pol;
}

`std::map *pol;` -- There is no need for this to be a pointer, and then later on use `new` and `delete`. All you need is `std::map pol;`. Also, can you clean up the formatting? It's all over the place, making it hard to read your code. — PaulMcKenzie, Jun 24 '16 at 21:30
Also, please post a [mcve], which means a `main` function. The reason is that I can easily break your program (all due to the pointer I mentioned) with a simple 2 or 3 line program. Also, operator+, -, and * should be returning a new `poly` object, not a reference. — PaulMcKenzie, Jun 24 '16 at 21:36
cout << (*pol)[exp]; I think that's gonna break if the value becomes 0, because you erase a few lines above. — Alex, Jun 24 '16 at 21:38
Thank you for the help. Next time I'll definitely have better formatting. — bigplop, Jun 25 '16 at 18:54

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

The line

map<int, int>* pol = new map<int, int>;

is the culprit. It creates a function local variable. It does not set the value of the member variable. As a consequence, the member variable of the same name remains uninitialized. Dereferencing that variable in other functions causes undefined behavior.

Change that line to:

pol = new map<int, int>;

As was suggested in one of the comments, I would strongly advise changing that member variable from a pointer to an object. You get automatic memory management from using an object. You don't need to worry about using new to allocate memory for it and using delete to deallocate memory used by it. Not only that, if you take on the job of allocating memory in your class, you need to know about The Rule of Three and make sure your class confirms to those rules.

If you are using a C++11 compiler, The Rule of Three becomes the The Rule of Five.

In GCC you can turn on: -Wshadow That should warn you in this situation. — Alex, Jun 24 '16 at 21:40
For C++11, the rule of three became a rule of five. That becomes a rule of zero for (among other things) classes that don't explicitly manage a resource (e.g. if `pol` becomes a `map` rather than a pointer to one). — Peter, Jun 24 '16 at 21:59

Iterating through map C++

1 Answers1