C++ convert iterator from map to object pointer

Question

Trying to find an object in a map, and set a member class pointer variable.

# Animations.h
#ifndef _ANIMATIONS_H
#define _ANIMATIONS_H

#include <map>
#include "Animation.h"

using namespace std;

class Animations {
  public:
    Animations();
    void add(Animation* animation, string name);
    void play(string name);

    Animation* animation;
    map<string, Animation*> animations;
};

#endif

# Animations.cpp
...
void Animations::play(string name) {
  std::map<string, Animation*>::iterator itr = animations.find(name);

  if (itr != animations.end()) {
    animation = itr->second;
  }
}

I've seen lots of examples with itr->second in couts and printing, which I've tested, works fine. However I need to convert it to an Animation * somehow, but don't know how. When I use the variable from above, it compiles, but the data is all messed up (frame should be 0 and instead is some huge int) so probably wrong memory address. I did verify the frame of the original Animation object is 0, and a pointer to it is 0 too before adding it to the map.

The closest I've come is this &(*itr); via https://stackoverflow.com/a/2160319/1183537 and what I'm doing is close to c++ Find map value and key although I want to assign the variable, not just print it out, which are the examples I usually find on map#find like https://cplusplus.com/reference/map/map/find/ and https://www.guru99.com/cpp-map-stl.html

Can anyone lend some help?

I'm newish to C++, and rusty on C, but programming for 10+ years, so I should be able to handle this.

edit: here's how i'm adding animations:

# Animations.cpp
void Animations::add(Animation* animation, string name) {
  animations[name] = animation;

  cout << "> Animations::add " << name << " frame: " << animation->frame << endl;
}

and here's how I'm testing the set animation pointer:

# Animations.cpp
void Animations::update() {
  cout << "> Animations::update" << endl;
  if (animation != NULL) {
    cout << "> Animations::update animation frame: " << animation->frame << " " << animation->width << "x" << animation->height << endl;
    animation->update();
  }
  cout << "> Animations::update done" << endl;
}

here's where I was creating the Animations and calling animations.add:

# Player.cpp
...
void Player::initAnimations(ALLEGRO_BITMAP* sheet) {
  Animation spark(FPS, true, true);
  Animation explosion(FPS / 3, true, true);

  spark.add(sheet, 34, 0, 10, 8);
  spark.add(sheet, 45, 0, 7, 8);
  spark.add(sheet, 54, 0, 9, 8);

  animations.add(&spark, "spark");
  animations.play("spark");
}

I've tried not using pointers, from the comment suggestions. But I'm getting error error: no matching constructor for initialization of 'Animation' around the place where I do

animation = itr->second;

not sure if it needs to be itr.second instead, I think I tried both.

`itr->second` is an `Animation*`. Your problem is almost certainly elsewhere. How are you creating the `Animation` objects that you store pointers to, for example? — Miles Budnek, Aug 08 '22 at 01:47
Looks like the `Animation*` points to garbage. Where are those animations coming from? Do they need to be pointers? If they do, why not a smart pointer? — Etienne de Martel, Aug 08 '22 at 01:55
okay, thanks I'll try showing how I'm using `animation`, testing it out — mswieboda, Aug 08 '22 at 01:56
@EtiennedeMartel I'll look into smart pointers, not familiar with those. Can I have a map of `Animation` and not pointers? I'd prefer that anyways, I'll try that too. — mswieboda, Aug 08 '22 at 01:57
Sure you can. It all depends on what's inside `Animation`. Usually, if it's not polymorphic, it's better to keep values around. — Etienne de Martel, Aug 08 '22 at 01:59
Two things I'd like to see: how are those `Animation` objects _created_ (i.e. what's passed to `add`), and also, do you initialise the `animation` member to `nullptr` at construction? Otherwise it's going to initially (i.e. before you call `play` once) contain garbage and therefore point to random stuff that, when interpreted as `Animation`, will look like a broken animation. — Etienne de Martel, Aug 08 '22 at 02:01
yes I intialized `animations = NULL` in the constructor. I'll try `nullptr` instead. I'm having errors while not using pointers, using `map animations;` instead: `error: no matching constructor for initialization of 'Animation'` when doing `itr.second` probably. — mswieboda, Aug 08 '22 at 02:06
As I expected: you're adding the address of a local variable (`spark` dies at the end of `initAnimations`, so any pointer to it becomes invalid). As for `animation = itr->second`, `animation` can stay a pointer, and then you can just do `animation = &itr->second`. — Etienne de Martel, Aug 08 '22 at 02:12
There's a popular myth: how to write C++ code without learning core C++ fundamental concepts? Answer: run a Google search and copy/paste the results. This only works if the found code's functionality is 100% identical to what's needed. This doesn't work if the found code is junk code, but without learning core C++ fundamentals there's no way to tell if it's junk. And if it's not a 100% match, knowledge of core fundamentals is needed to change it accordingly, but a Google search won't help with this, now, by definition. — Sam Varshavchik, Aug 08 '22 at 02:15
I'm not copy/pasting @SamVarshavchik, I'm actively learning how to use map's and vectors right now. And clearly I misunderstood local pointers would become invalid. Perhaps I should invest in a C++ book — mswieboda, Aug 08 '22 at 02:19
okay @EtiennedeMartel I got it working with your suggestions. I just also needed to make a default `Animation()` constructor (i had a custom one). I'll make an answer. Thanks for all your help! — mswieboda, Aug 08 '22 at 02:21
I think this is an excellent idea: [investing in a good C++ textbook](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — Sam Varshavchik, Aug 08 '22 at 02:26
This site might give you a start while waiting for books to arrive : https://www.learncpp.com/. A tip : recent C++ usually doesn't have all that much new/delete and raw pointers in it. (but uses std::make_unique/std::make_shared). And containers either have (moveable) objects in them (no pointers at all) or unique/shared pointers. This will avoid a lot of bugs because of null pointers and/or forgetting to delete them (memory leaks) — Pepijn Kramer, Aug 08 '22 at 02:55

score 1 · Accepted Answer · answered Aug 08 '22 at 02:30

I switched to non pointers in the map

# Animations.h

map<string, Animation> animations;
Animation* animation;

while leaving the animation a pointer. Then thanks to @EtiennedMartel's comment I used &itr->second to set the Animation* animation pointer. The other thing I needed to do was to add a default Animation() constructor, which I originally didn't have, which was needed from the map/iterator.

# Animations.cpp

void Animations::play(string name) {
  std::map<string, Animation>::iterator itr = animations.find(name);

  if (itr != animations.end())
    animation = &itr->second;
}

C++ convert iterator from map to object pointer

1 Answers1