0

Please note that i'm a newbie with classes. Here's an idea of what i want (and need to make my code more clean and versatile)

Inside my function StartupRoutine(std::map<std::string, MyClass*>& mymap):

MyClass myobj[2];
myobj[0] = new MyClass(/* constructor parameters */);
myobj[1] = new MyClass(/* constructor parameters */);

/* do some stuff and call some setters */

mymap.insert(std::pair<std::string, MyClass*>("something", myobj)); /* myobj should be equal to &myobj[0] right? */

I'm calling that from the main. I want the classes to stay until i call delete.

I want to do that in order to easily access class methods with both objects with an offset like: mymap["something"] and mymap["something"]+1.

And i'd like to use the same map for single objects and arrays of 2 or 4 objects, for several reasons,

I have tried many many ways and google for hours. I tried something like the example above. Then i tried to initialize myobj as a single pointer:

MyClass* myobj;
myobj = new MyClass(/* constructor parameters */);
myobj+1 = new MyClass(/* constructor parameters */);

which i'm pretty sure it's really wrong because i'm trying to access a memory address i'm not supposed to.

EDIT: Thanks everyone for the help, i'm going to answer the comments here: about the new statement you guys are right i'm an idiot and forgot about that after hours of trying. Here a more detailed explanation of what i'm doing: I'm making a simple game with SDL and i have my class for the textures. I want to map my texture (probaly there are better ways to handle them but that's the best solution i came up with to make my code more versatile (and better looking)). So most of the textures are in the map will be a single object, but in some cases, for the animations, i use a seamless texture and i need more instances of the same image but with different coordinates (which are both attributes of the class). I'd like to handle normal textures as single objects and seamless textures as arrays of 2 or 4 objects in the same map. i know i can simply map them individually like map["water1"] and map["water2"] but to me it's very very ugly and i'm trying to find a better solution.

BTW I'm doing all of this for educational purposes. PS: sorry for my absence but in weekends i do work.

SECOND EDIT: what if i use malloc and instead of new and do this:

MyClass* MyObj = (MyClass*)malloc(sizeof(MyClass)*2);
*MyObj = MyClass(/* constructor parameters */);
*(MyObj+1) = MyClass(/* constructor parameters */);
mymap.insert(std::pair<std::string, MyClass*>("something", MyObj));

for some reason it crashes when i try to call a class method like:

mymap["something"]->MyMethod();
Borbi
  • 25
  • 5
  • It's not clear what you actually want. Do you want `myobj` to be an array of `MyClass*` pointers, instead of `MyClass` objects? And then instead of `mymap.insert(std::pair("something", myobj));` you could just do `mymap["something"] = myobj[0];`? – Nathan Pierson Jun 01 '23 at 22:00
  • First off, you don't use new to initialize objects in an array of said objects. (You might use it to initialize pointers in an array of pointers to such objects.) There is also an implicit issue of lifetime here. Is your array guaranteed to live at least as long as your map? – Avi Berger Jun 01 '23 at 22:00
  • 6
    Note that `new` is an advanced tool, that most programmers don't use in their work (assuming they're sufficiently educated to understand that). Though, of course, you should make yourself familiar with how to use it. Without `new`, this would become just `std::map>`. – HolyBlackCat Jun 01 '23 at 22:11
  • The key is that `new MyClass` returns a `MyClass *`. Thus, you COULD do `MyClass * myobj[2];`, and your first sample would work. It's usually better to do `vector myobj;` then `myobj.emplace_back(/*Constructor parameters*/);`. – Tim Roberts Jun 01 '23 at 22:43
  • `new MyClass(/* constructor parameters */);` is grossly simplified concept taken from 20-year-old book. Actually it's `new (type) /* initializer*/;` or `new type /* initializer*/;` or `new (placement-params)(type) /* initializer*/;` or `new (placement-params) type /* initializer*/;` Note that initializer can have braces instead of parents and that may or may not have different meaning – Swift - Friday Pie Jun 03 '23 at 18:22
  • Note: Only under rare, very controlled circumstances should you ever use `malloc` and family on a class. `malloc` only provides memory. It does not provide an object. Constructors do not get run. – user4581301 Jun 05 '23 at 02:52

2 Answers2

1

The immediate issue with your approach is that you are trying to store MyClass* values in a MyClass array. The new statement returns a pointer, not a direct object reference. You could use one of the following:

Use an array of pointers.

You can do it pretty much the way you are currently trying to do it by adding an extra layer of memory accesses.

MyClass* myobj[2];
myobj[0] = new MyClass(...);
myobj[1] = new MyClass(...);

/* ... */

mymap.insert(std::pair<std::string, MyClass**>("something", myobj));

To get a pointer to the ith element in your array, you can use mymap["something"] + i.

However, doing it this way is messy and can cause a lot of headaches and segfaults if you are interacting with the double-pointers very much. I would recommend the latter method.

Use a vector.

Include the header <vector> to use the standard library vectors. Vectors are dynamic-length data structures which come with handy features like built-in range checking. You can new up a vector to make it stick around after the function scope ends, just make sure you delete it after you're done (and after you delete the MyClass* values that you put in it).

std::vector<MyClass*>* myobj = new std::vector<MyClass>();
myobj->push_back(new MyClass(...));
myobj->push_back(new MyClass(...));

/* ... */

mymap.insert(std::pair<std::string, std::vector<MyClass*>* >("something", myobj));

Then, you access the ith element in the vector, which is a pointer to a MyClass object, with mymap["something"]->at(i).

Since you said you'll be using collections of different sizes, this will probably save you a lot of trouble. Any time you try to access an element outside of a vector's range, it will throw an error. If you make the same mistake using an array and mymap["something"] + i, it will just grab whatever garbage memory is sitting next to that array in memory and not know the difference.

Unless you have some specific reason not to, I would very much recommend using a vector instead of an array and memory arithmetic.

Ethan Maness
  • 161
  • 8
  • Yes if possible i want to avoid using double pointers. For the vector honestly i don't know, most of the objects are going to be a single instance, just a few are going to be arrays. Is it still a good idea to use vectors in my case? Is it a waste of resources using most vectors in the map to store single objects? – Borbi Jun 03 '23 at 16:56
  • This answer is mostly right. There are almost no good reasons to dynamically allocate `std::vector`, and this isn't one of them. Stick the `vector`s directly into the map. And since a `vector` is basically a pointer to an array with resource management built in, there's usually no point to having a `vector` of pointers. – user4581301 Jun 05 '23 at 02:56
  • @Borbi That is a good question--if runtime is going to be a critical priority, it may be worth it to use the hacky double pointer method. Like all abstractions, vectors will incur a computational cost, although for vectors in particular it is quite small. – Ethan Maness Jun 05 '23 at 21:24
  • @user4581301 I thought that, unless the `new` statement was used, an object was destroyed when the scope of the function (in this case `StartupRoutine`) ended. Given the name of the function, it seems like these vectors would need to be accessed outside of its scope. Same logic for the MyClass pointers. – Ethan Maness Jun 05 '23 at 21:27
  • In the case of `vector` and `map`, the `new`, or something like it, is hidden deep in the guts of the implementation surrounded by [RAII](https://stackoverflow.com/q/2321511/4581301). If you put a `vector` in the `map`, the `map` owns it and will look after it until it's removed from the `map`. and anything you put in the `vector` is likewise looked after. When the map goes out of scope it'll take all of the `vector`s and `MyClass`es with it. – user4581301 Jun 05 '23 at 21:36
  • If you need to preserve some of the items in the `map` from joining the `map` in death, you can `move` the items, transferring ownership into a different variable to hold onto them longer. – user4581301 Jun 05 '23 at 21:36
  • General rule of thumb is if you see code dynamically allocating a library container you're either looking at a bug-in-the-offing or a very interesting problem with an even more interesting solution. – user4581301 Jun 05 '23 at 21:38
0
MyClass myobj[2];

defines an array of two MyClass instances. Not pointers to instances, so using

myobj[0] = new MyClass(/* constructor parameters */);

is not possible. new provides a pointer to a dynamically allocated MyClass and a pointer to a MyClass isn't a Myclass. You cannot assign them. Plus you have two MyClasses here: the assignee and the brand new one allocated by new, and someone would have to give the newed back with delete eventually to prevent a leak.

When you define an array it immediately constructs every element in the array. If the elements to not have a default constructor (a constructor with no parameters or nothing but defaulted parameters), the array can only be defined with a list.

Say we have something simple like

class MyClass
{
public:
    MyClass(int ,int);
};

which requires two integers. We can provide those integers with a list

MyClass myobj[2] = {
    {1, 2},
    {3, 4}
};

We could use existing variables in the list

MyClass myobj[2] = {
    {a, b},
    {c, d}
};

and we can also get them from a function

MyClass myobj[2] = {
    {get_int(), get_int()},
    {get_int(), get_int()}
};

I'm probably missing some other sneaky tricks here, but you probably don't need them for this job.

Now you can

mymap.insert(std::pair<std::string, MyClass*>("something", myobj));

but you have to make absolutely certain that myobj outlives mymap. Do not do stuff like

void function_of_doom(map<<std::string, MyClass*>> & mymap)
{
    MyClass myobj[2] = {
        {a, b},
        {c, d}
    };
    mymap.insert(std::pair<std::string, MyClass*>("something", myobj));
}

where myobj will go out of scope before mymap.

Side note:

Don't try stuff like

myobj[0] = *new MyClass(/* constructor parameters */);

even if the compiler allows it. new MyClass(/* constructor parameters */) created a brand new, dynamically allocated MyClass floating somewhere in storage and provided a pointer to it so you can find it. the * dereferences the pointer, getting the pointed-at object, and myobj[0] = copies that new object into myobj[0]. All of this is perfectly legal and it looks like you get what you want. But you lost the pointer to that new object, and without the pointer you don't know where it is, making it awesomely hard to track down so you can give the memory back with delete. If you're going to dynamically allocate and object, you always need to keep a reference to the object so you can free it when when you're done. This is harder than it looks once programs get larger, so the general rule of thumb is to use new extremely rarely.

OK. So we want the function of doom. That means we need need dynamic scope. Yes you can get that with new, but like I said, use it rarely. We don't need it here.

std::map<std::string, std::vector<MyClass>> mymap;

maps strings to a vector of MyClasses. There are a hole bunch of different ways we could put stuff in, but the easiest is probably

std::vector<MyClass> myobj = {
    {a, b},
    {c, d}
};
mymap["something"] = myobj;

all of the dynamic allocation performed here is performed for you and protected by RAII-observing classes. You can add and remove MyClasses from the vectors and you can add and remove vectors from the map.

eg:

mymap["something"].emplace_back(5,6);

to add and to remove

auto found = // find item
mymap["something"].erase(found);

To remove the entire "something"

mymap.erase("something"); 

So long as an object remains in in the owning container, it will continue to live. Remove an item and it's automatically destroyed and released for you. No messing around with pointers or new and delete and no chance of a memory leak.

The function of doom now looks like

void function_of_NOT_doom(std::map<std::string,
                          std::vector<MyClass>> mymap)
{
    std::vector<MyClass> myobj = {
        {a, b},
        {c, d}
    };
    mymap["something"] = myobj;
}
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • Yes i want exactly to make the function of doom (my startup function) AND i want my objects to be persistent until i delete them and free the memory. Right now i create all the objects in that function, but in the future i want to free some and add new ones as i need with some functions i'm going to make. That's the reason i'd like to use pointers – Borbi Jun 03 '23 at 17:03
  • @Borbi here's a completely managed, leak-free solution. – user4581301 Jun 05 '23 at 03:20
  • thanks a lot i think this problem is solved now, it compiles well and do not crash on runtime, now i have a different problem to solve, better get back to work! – Borbi Jun 05 '23 at 10:10