-1

Let's say I want a private pointer to a map in my class. And I want it to be pointing to NULL if the argument of my constructor (a boolean) is true. Is it the correct way of doing it ? (if my boolean is false, I add a first element in my map)

using namespace std;
#include <map>

class Class {
 public:
  Class(bool a)
   : _myMap(0) {
    if (a) {
      _myMap = new map<int, bool>(); // initialized to NULL
    } else {
      (*_myMap)[0] = true; // added a first key-value to map
    }
  }
  ~Class() {
   delete _myMap;
 }
 private: 
  map<int, bool>* _myMap;
}

EDIT Here is how I fixed my problem :

using namespace std;
#include <map>

class Class {
 public:
  Class(bool a)
   : _myMap(0) { // map initialized to NULL
    if (a) {
      _myMap = new map<int, bool>();
    }
  }
  ~Class() {
   delete _myMap;
  }
  void addValue(bool b, int i) {
     if (_myMap != 0) {
        (*_myMap)[i] = b;
     }
  }
 private: 
  map<int, bool>* _myMap;
}

To answer folks that ask me why I needed a pointer to a map instead of a simple map : when I use my component Class, I don't want to add values if a (used in the constructor) is false, i.e if my map is pointing to NULL.

klaus
  • 754
  • 8
  • 28
  • 2
    First thing first. Why pointer? Why dynamic allocation? –  May 04 '17 at 06:06
  • I want a pointer because I want to be able to check if my map is pointing to NULL or not. I was not sure about the correct way of initialization of my pointer to map. – klaus May 04 '17 at 06:08
  • Then, what exactly in your mind do you mean by "NULL"? And why exactly do you want to "pointing to NULL"? –  May 04 '17 at 06:09
  • I want a map either pointing to NULL (meaning there is no elements in it), either not pointing to NULL (meaning there are elements in it). – klaus May 04 '17 at 06:22
  • 2
    @klaus A map doesn't point. Pointers point. A null pointer is not the same as a map with no elements in it. – melpomene May 04 '17 at 06:23
  • 1
    You don't need a pointer to a map. You need to be able to handle maps with no elements. That's an [XY problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). – n. m. could be an AI May 04 '17 at 06:39

2 Answers2

0

: _myMap(0) initializes the pointer to NULL.

Then

_myMap = new map<int, bool>(); // initialized to NULL

allocates a map and points _myMap at it. That's the opposite of a null pointer. The return value of new is guaranteed to not be NULL.

In the else branch, you do

(*_myMap)[0] = true;

This has undefined behavior. You're dereferencing _myMap, which is a null pointer.

In conclusion: No, this is not correct.


You could fix this particular bit of code by doing:

if (!a) {
  _myMap = new map<int, bool>();
  (*_myMap)[0] = true; // added a first key-value to map
}

But you'd still have to write a correct copy constructor and assignment operator, and the whole thing feels icky. Consider using a "smart pointer" type instead.


Based on your comments, you seem to want a (possibly empty) map in your class. That can be done much easier without pointers:

class Class {
 public:
  Class(bool a)
  {
    if (!a) {
      _myMap[0] = true; // added a first key-value to map
    }
  }

 private: 
  map<int, bool> _myMap;
}

Now we don't need any dynamic memory allocation or custom destructor.

melpomene
  • 84,125
  • 8
  • 85
  • 148
  • Thank you. Then I will keep _myMap(0) in the constructor and delete the new. But if I want to add elements to my map, I need to dereference `_myMap` as I did, right ? – klaus May 04 '17 at 06:20
  • 2
    @klaus So you want the pointer to be pointing to NULL but you don't know what that means. It seems you don't understand what a pointer is either, so maybe you should reconsider whether you need the code at all. – juanchopanza May 04 '17 at 06:21
0

Ignoring for a moment the rationale for making life harder than it needs to be ...

  Class(bool a)
   : _myMap(0) {
    if (a) {

      // You are not doing what your comment says
      // you want to do.
      _myMap = new map<int, bool>(); // initialized to NULL
    } else {

      // This is bad.
      // myMap is still a null pointer. You are dereferencing
      // a null pointer. Only bad things can happen with this.
      (*_myMap)[0] = true; // added a first key-value to map
    }
  }

You can use something along the lines of:

  Class(bool a)
   : _myMap(nullptr) {  // Use nullptr for pointers.
    if (a) {
      // There is nothing to do.
    } else {
      // Allocate memory for myMap
      _myMap = new map<int, bool>(); // initialized to NULL

      // Now you can dereference the pointer.
      (*_myMap)[0] = true; // added a first key-value to map
    }
  }

Don't forget the Rule of Three when you store a pointer to heap memory in your object.

Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270