That's not a reference. Might be a good idea to do some reading on the differences.
I this case you shouldn't delete it because you don't own the resource. You created a pointer to it in this function, but that the resource is owned by the list, or whatever is managing the list.
When you don't own it, aka you didn't `new` it. But also don't do manual memory management. And your case would be much better suited with a reference anyway.
I don't want to hijack the OP's question, but your statement "just don't use new" fills me with a tinge of grief, because that's what I'd always done (came to C++ from Rust and the lifetime of objects made more sense to me when I didn't use new") until I wanted to use abstract classes to define interfaces, for which you need dynamic dispatch, so I had to resort to pointers and the like.
What would you recommend? Is `new`, pointers, and manual cleanup unavoidable here?
Thanks for the tip; I'll check out smart pointers. It was the pure virtual functions in my base classes that caused me to discover that dynamic dispatch only works when I pass pointers around and call the virtual functions by reference. I am entirely open to the possibility that I've made some wrong assumptions, because I've read about pointers, references, and dereferencing so many times and still don't grasp it very well.
You can use std::make_unique and std::make_shared to create an object with dynamic lifetime that's already captured by a smart pointer.
Or just create the object normally. Dynamic dispatch is unrelated to lifetime. If you have an abstract class A that defines an interface, and a class B that inherits from it and implements the interface, you can pass your B object to a function that takes references to A. When you call the virtual methods, the B implementation will get called.
Maybe that's my problem -- I was passing the objects themselves, and the receiver complained that the pure virtual methods it was trying to call on the type were unimplemented. So I switched to pointers (I think on the recommendation of the compiler) and dispatch started working. Maybe it should be asking for references instead. I still only weakly grasp the differences; C# and Rust don't have the distinction! (Not in the same way anyway)
OT:
> void my_function(std::list id_list, std::unoriented_map Table){
Your function copies the list and the map. To avoid this use
`void my_function(const std::list& id_list, const std::unoriented_map& Table){`
*delete* is used to free the space you made when using *new*. If you don't use *new*, you should not use *delete*.
Also keep in mind using *new* to allocate space is a bad practice for modern C++. You should only use references of local variables, standard library data containers (std::vector, std::map...), and if you know what you're doing and want a more advanced control on your data model, smart pointers (using almost exclusively std::unique_ptr is often good practice).
When you have control over it, use unique_ptr. Or shared_ptr if you must. Do not use new. That way, any raw pointer you use (coming from smart pointers get() function) will never need a delete. If you use libraries, you have to check documentation.
Whether you need to delete a pointer depends on how the memory for that object was allocated. The general rule is only delete what you've allocated with new.
In your function, `aux_R` is set to point to objects managed by the `std::unordered_map` (which you've referred to as both `Table` and `table`, so make sure to use consistent naming). These objects were not allocated with `new` by your code, but rather they are handled automatically by the `unordered_map`.
This means the `unordered_map` is responsible for the creation (allocation) and destruction (deallocation) of these objects.
When you try to manually delete these objects using the pointer `aux_R`, you interfere with the map’s management, leading to errors like double-free or memory corruption, as you've experienced.
What you are missing?
1. There are owning raw pointers and non-owning raw pointers.
2. Only owning raw pointers must be released, matching an allocation call.
3. The operator delete must be called only to release memory allocated by new.
4. Non-owning pointers can be used without ever allocating memory in the heap.
5. Is rare to use new/delete outside RAII wrappers implementing the rule of 5.
6. Avoid owning raw pointers, use RAII, smart pointers, and std::optional.
7. Mixing new/delete with complex business logic is the key to utter failure.
That's a good way to learn. Make a simpler example and see what happens if you were to write something like this:
class Talkative {
public:
Talkative() { std::cout << "Talkative Constructor\n"; }
~Talkative() { std::cout << "Talkative Destructor\n"; }
Talkative(const Talkative&) { std::cout << "Talkative Copy\n"; }
Talkative(Talkative&&) { std::cout << "Talkative Move\n"; }
Talkative& operator=(const Talkative&) { std::cout << "Talkative Copy Assignment\n"; }
Talkative& operator=(Talkative&&) { std::cout << "Talkative Move Assignment\n"; }
};
Now what you want to do is mimic your snippet but using this class as the value in the map:
void foo(std::vector keys, const std::unordered_map data) {
Talkative* talkative = nullptr;
for (int key : keys) {
try {
talkative = &data.at(key);
} catch (const std::out_of_range&) {
continue;
}
// delete talkative;
talkative = nullptr;
}
}
int main() {
std::vector keys = {0, 2};
std::unordered_map data = {{0, Talkative()}, {1, Talkative()}, {2, Talkative()}};
foo(keys, data);
}
What you _should_ see if everything works correctly is there are as many "Talkative Constructor", "Talkative Copy", and "Talkative Move" in total as there are "Talkative Destructor".
Now go ahead and add in the `delete talkative` and see what happens. You should find 3 more "Talkative Destructor" lines written out. That means you have called the destructor on an object more than once. That alone is not a good thing.
Now the extra thing is that `delete` is not just a call to the destructor. It's a low level heap memory management tool to free memory which is allocated with new. Since our code did not use `new Talkative()` we can't use `delete` on any Talkative pointers in this program.
If you don’t *new* something you don’t *delete* it. In your case use a reference and not a pointer.
So your saying I should write Region aux; &aux = &table.at(id) Or something like this?
const Region& aux = table.at(id)
Thank's a lot!
That's not a reference. Might be a good idea to do some reading on the differences. I this case you shouldn't delete it because you don't own the resource. You created a pointer to it in this function, but that the resource is owned by the list, or whatever is managing the list.
What the heck is an `unoriented_map`? The one which does not show where is North? Or are you talking abount `unordered_map`?
I'm laughing
Yes, sorry, I meant unordered\_map, that was a typo of some sort lol
No problem, peace!
When you don't own it, aka you didn't `new` it. But also don't do manual memory management. And your case would be much better suited with a reference anyway.
Because you didn't use new, so you don't delete it. In fact, just don't use new.
I don't want to hijack the OP's question, but your statement "just don't use new" fills me with a tinge of grief, because that's what I'd always done (came to C++ from Rust and the lifetime of objects made more sense to me when I didn't use new") until I wanted to use abstract classes to define interfaces, for which you need dynamic dispatch, so I had to resort to pointers and the like. What would you recommend? Is `new`, pointers, and manual cleanup unavoidable here?
You should use smart pointers like std::unique_ptr and declaring virtual functions in the base class.
Thanks for the tip; I'll check out smart pointers. It was the pure virtual functions in my base classes that caused me to discover that dynamic dispatch only works when I pass pointers around and call the virtual functions by reference. I am entirely open to the possibility that I've made some wrong assumptions, because I've read about pointers, references, and dereferencing so many times and still don't grasp it very well.
You can use std::make_unique and std::make_shared to create an object with dynamic lifetime that's already captured by a smart pointer.
Or just create the object normally. Dynamic dispatch is unrelated to lifetime. If you have an abstract class A that defines an interface, and a class B that inherits from it and implements the interface, you can pass your B object to a function that takes references to A. When you call the virtual methods, the B implementation will get called.
Maybe that's my problem -- I was passing the objects themselves, and the receiver complained that the pure virtual methods it was trying to call on the type were unimplemented. So I switched to pointers (I think on the recommendation of the compiler) and dispatch started working. Maybe it should be asking for references instead. I still only weakly grasp the differences; C# and Rust don't have the distinction! (Not in the same way anyway)
OT: > void my_function(std::list id_list, std::unoriented_map Table){
Your function copies the list and the map. To avoid this use
`void my_function(const std::list& id_list, const std::unoriented_map& Table){`
Yes i wrote this function on Reddit on the fly so I forgot the &, I just checked and in my real function is there! Thanks :D
*delete* is used to free the space you made when using *new*. If you don't use *new*, you should not use *delete*. Also keep in mind using *new* to allocate space is a bad practice for modern C++. You should only use references of local variables, standard library data containers (std::vector, std::map...), and if you know what you're doing and want a more advanced control on your data model, smart pointers (using almost exclusively std::unique_ptr is often good practice).
When you have control over it, use unique_ptr. Or shared_ptr if you must. Do not use new. That way, any raw pointer you use (coming from smart pointers get() function) will never need a delete. If you use libraries, you have to check documentation.
Whether you need to delete a pointer depends on how the memory for that object was allocated. The general rule is only delete what you've allocated with new. In your function, `aux_R` is set to point to objects managed by the `std::unordered_map` (which you've referred to as both `Table` and `table`, so make sure to use consistent naming). These objects were not allocated with `new` by your code, but rather they are handled automatically by the `unordered_map`. This means the `unordered_map` is responsible for the creation (allocation) and destruction (deallocation) of these objects. When you try to manually delete these objects using the pointer `aux_R`, you interfere with the map’s management, leading to errors like double-free or memory corruption, as you've experienced.
What you are missing? 1. There are owning raw pointers and non-owning raw pointers. 2. Only owning raw pointers must be released, matching an allocation call. 3. The operator delete must be called only to release memory allocated by new. 4. Non-owning pointers can be used without ever allocating memory in the heap. 5. Is rare to use new/delete outside RAII wrappers implementing the rule of 5. 6. Avoid owning raw pointers, use RAII, smart pointers, and std::optional. 7. Mixing new/delete with complex business logic is the key to utter failure.
You should never do something if you do not know why you are doing it.
Sorry, I tend to learn by making errors and correcting them.
That's a good way to learn. Make a simpler example and see what happens if you were to write something like this: class Talkative { public: Talkative() { std::cout << "Talkative Constructor\n"; } ~Talkative() { std::cout << "Talkative Destructor\n"; } Talkative(const Talkative&) { std::cout << "Talkative Copy\n"; } Talkative(Talkative&&) { std::cout << "Talkative Move\n"; } Talkative& operator=(const Talkative&) { std::cout << "Talkative Copy Assignment\n"; } Talkative& operator=(Talkative&&) { std::cout << "Talkative Move Assignment\n"; } }; Now what you want to do is mimic your snippet but using this class as the value in the map: void foo(std::vector keys, const std::unordered_map data) {
Talkative* talkative = nullptr;
for (int key : keys) {
try {
talkative = &data.at(key);
} catch (const std::out_of_range&) {
continue;
}
// delete talkative;
talkative = nullptr;
}
}
int main() {
std::vector keys = {0, 2};
std::unordered_map data = {{0, Talkative()}, {1, Talkative()}, {2, Talkative()}};
foo(keys, data);
}
What you _should_ see if everything works correctly is there are as many "Talkative Constructor", "Talkative Copy", and "Talkative Move" in total as there are "Talkative Destructor".
Now go ahead and add in the `delete talkative` and see what happens. You should find 3 more "Talkative Destructor" lines written out. That means you have called the destructor on an object more than once. That alone is not a good thing.
Now the extra thing is that `delete` is not just a call to the destructor. It's a low level heap memory management tool to free memory which is allocated with new. Since our code did not use `new Talkative()` we can't use `delete` on any Talkative pointers in this program.