0

I'm trying to (somehow) cache multiple iterations on a std::map when I need to insert multiple values by using this method:

enum PROPERTY_TYPE
{
    TYPE_BOOLEAN,
    TYPE_INTEGER,
    TYPE_UNSIGNED,
    TYPE_FLOAT,
    TYPE_STRING
};
struct Property {
    Property(PROPERTY_TYPE type)
        : type(type), bool_val(false), integer_val(0), unsigned_val(0), float_val(0.0f), string_val("")
        {  }
    PROPERTY_TYPE       type;
    bool                bool_val;
    int                 integer_val;
    unsigned int        unsigned_val;
    float               float_val;
    std::string         string_val;
};
std::map< std::string, Property* >          _Properties;

void pushBool(std::string key, bool value)
{
    std::pair< std::map< std::string, Property* >::iterator, bool > pItr;
    pItr = _Properties.insert( std::pair< std::string, Property* >(key, new Property(TYPE_BOOLEAN)) );
    // If the value doesn't exist then it's created automatically or the existion one is used.
    pItr.first->second->bool_val = value;
    pItr.first->second->integer_val = value ? 1 : 0;
    pItr.first->second->unsigned_val = value ? 1 : 0;
    pItr.first->second->float_val = value ? 1.0f : 0.0f;
    pItr.first->second->string_val = value ? "true" : "false";
}

This is the fastest and safest way I can get it to work so far. However I'm interested in one little thing and that's the insert() function. When I call new Property(TYPE_BOOLEAN) I clearly create a new Property() that nobody controls it. And from what I read, std::map doesn't call delete on pointers when discard. And when I use this method of inserting the insert function uses the existing value if it already exists. So, Who deletes that newly created Property() if a value already exists and it's used instead of the new one?

    pItr = _Properties.insert( std::pair< std::string, Property* >(key, new Property(TYPE_BOOLEAN)) );

Would this method be a waste of memory by introducing memory leaks?

SLC
  • 2,167
  • 2
  • 28
  • 46

2 Answers2

3

Yes, that line can cause a leak.

Either store smart pointers, or check for existance before inserting. I would advise smart pointers. unique_ptr probably, to indicate ownership.

In this particular case, free store allocation is a bad idea, but I assume the real problem has a reason to store items there.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
1

You're right - std::map doesn't buy in to the question of who deletes any pointers it stores - it expects the client code to take responsibility, or store a type that does that inherently. The simplest and often best way to do that - the way you'd use unless you'd seen a clear reason to do otherwise - is to store a "value semantic" type - one that takes care of its own life, and can be copied intuitively, behaving much like an int or other inbuilt type in that regard.

In your case, there's ostensibly no reason to use pointers at all... just

std::map< std::string, Property > _Properties;

If you don't care whether you're inserting a new element or updating an old one:

Property& p = _Properties[key];
p.bool_val = value;
p.integer_val = ... etc

Separately, if the idea is that a Property conceptually stores any one of those types of data, you may find boost::variant useful: http://www.boost.org/doc/libs/1_55_0/doc/html/variant.html

Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • You're right, I should use std::map< std::string, Property > but I seem to forgot that I'm not using a vector that resizes it's self when inserting (nasty habit) so that's the reason I started with pointers. I've never used maps so far and from what I've read so far they don't delete values when resizing like vectors. Thank you for your help. I'll do some benchmarks and see how it goes :) – SLC Feb 01 '14 at 15:45
  • 1
    @SanduLiviuCatalin Even when using a std::vector, I do not store pointers, but objects managing memory at copy/assignment/destruction (An exemption is a well encapsulated container) –  Feb 01 '14 at 15:54