dynamic_cast

This is a discussion on dynamic_cast within the C++ Programming forums, part of the General Programming Boards category; I'm trying to downcast from a GameEvent* to a Shot* Code: void Game::addEvent(GameEvent* event) { boardTime=&(Time)(event->getTime()); int timePeriod=event->getTime().getPeriod(); if (timePeriod==2 ...

  1. #1
    Registered User
    Join Date
    Oct 2009
    Posts
    44

    dynamic_cast

    I'm trying to downcast from a GameEvent* to a Shot*

    Code:
    void Game::addEvent(GameEvent* event) {
      boardTime=&(Time)(event->getTime());
      int timePeriod=event->getTime().getPeriod();
      if (timePeriod==2 && halfTimeouts==0) {
        halfTimeouts=1;
        homeTimeouts=3;
        awayTimeouts=3;
      }
      if (event->getType().compare("shot")==0) {
        Shot* shot=dynamic_cast<Shot*>(event);
        if (shot != 0) {
          cout << shot->getShooter()->getName()<<endl;
        }
        else {
          cout << "This object is not of type Shot"<<endl;
        }
        if (shot->getSuccessful()==1) {
          if (shot->getShooter()->getTeam()==homeTeam) {
            homeScore+=shot->getType();
          }
          else {
            awayScore+=shot->getType();
          }
        }
        events.push_back(event);
      }
    }
    I'm getting a segfault because when I call getShooter() there's no information to get. I was wondering what I'm doing wrong.
    Last edited by TIMBERings; 04-22-2010 at 11:30 AM.

  2. #2
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,794
    Maybe because that bit of code is outside where you test if the event pointer is a Shot pointer or not?
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  3. #3
    Registered User
    Join Date
    Oct 2009
    Posts
    44
    That's what the
    Code:
    (event->getType().compare("shot")==0)
    is for. Each GameEvent has a variable string type which tells me that it's a shot. Also, I'm know why the segfault exists, but I'm wondering why shot is never being initialized.
    Last edited by TIMBERings; 04-22-2010 at 11:30 AM.

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,448
    I think what hk_mp5kpdw means is that you should have written:
    Code:
      if (event->getType().compare("shot")==0) {
        Shot* shot=dynamic_cast<Shot*>(event);
        if (shot != 0) {
          cout << shot->getShooter()->getName()<<endl;
          if (shot->getSuccessful()==1) {
            if (shot->getShooter()->getTeam()==homeTeam) {
              homeScore+=shot->getType();
            }
            else {
              awayScore+=shot->getType();
            }
          }
          events.push_back(event);
        }
        else {
          cout << "This object is not of type Shot"<<endl;
        }
      }
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,794
    If the purpose of that test is to definitively determine that event is a shot event then do you really need the if/else test after the dynamic cast? Would the bit in green below ever get executed if we've already determined that we have a shot object?

    Otherwise, if not, then you'd definitely need to be careful here:
    Code:
      if (event->getType().compare("shot")==0) {
        Shot* shot=dynamic_cast<Shot*>(event);
        if (shot != 0) {
          cout << shot->getShooter()->getName()<<endl;
        }
        else {
          cout << "This object is not of type Shot"<<endl;
        }
        if (shot->getSuccessful()==1) {
          if (shot->getShooter()->getTeam()==homeTeam) {
            homeScore+=shot->getType();
          }
          else {
            awayScore+=shot->getType();
          }
        }
        events.push_back(event);
      }
    If the dynamic cast happens to fail, you simply print a message (the green bit) and then go on and attempt to use the pointer (the bit in red above) as if it succeeded. Maybe that red bit belongs with the blue bit.

    [edit]Yeah, that what I was talking about. Thanks laserlight[/edit]
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  6. #6
    Registered User
    Join Date
    Oct 2009
    Posts
    44
    I got it, turns out i copied a similar code to make this one and forgot to change "foul" to "shot", I was looking at it for 30 minutes without finding it.

    Thank you though.

  7. #7
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    By the way, why do you have what amounts to two type checks? First you check the event type string against "shot" and then you check the event object type against Shot. Shouldn't those two things be guaranteed to occur at the same time, so that you can use static_cast instead of dynamic_cast?
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  8. #8
    Super Moderator VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,590
    I would agree that static_cast would be safe here since you know the type of event and therefore know the data type. While dynamic_cast ensures type safety it is not needed here b/c you know the type and therefore can use static_cast.

  9. #9
    Registered User
    Join Date
    Jun 2005
    Posts
    6,208
    Quote Originally Posted by laserlight View Post
    I think what hk_mp5kpdw means is that you should have written:
    Personally, I'd consider doing it the other way around.

    Code:
        Shot* shot=dynamic_cast<Shot*>(event);
        if (shot != 0 && event->getType().compare("shot")==0) {
          cout << shot->getShooter()->getName()<<endl;
          if (shot->getSuccessful()==1) {
            if (shot->getShooter()->getTeam()==homeTeam) {
              homeScore+=shot->getType();
            }
            else {
              awayScore+=shot->getType();
            }
          }
          events.push_back(event);
        }
        else {
          cout << "This object is not of type Shot"<<endl;
        }
      }
    All that's happening here is that there are multiple checks on the event type. If both checks are required (i.e. one can give a false positive that is caught by the other) then make it more obvious in how you do the test.

    In practice, I'd prefer to do the dynamic_cast<> check first. That's a philosophical thing: use language features rather than hand-rolled code to achieve a desired effect if possible. More chance - subject to using a suitably verified compiler - of the code doing what is intended in various operating conditions. I therefore tend to look askance at classes that implement some form of type checking mechanism.

    There may be a performance argument for doing it the other way around (dynamic_cast<> is not necessarily all that efficient at run time), but that sort of decision needs to be made based on profiling, etc.

    I'd also be looking closely to see if the getType->compare() parts are even needed. That suggests holes in the class design.
    Last edited by grumpy; 04-23-2010 at 05:40 PM.
    Right 98% of the time, and don't care about the other 3%.

  10. #10
    Super Moderator VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,590
    That suggests holes in the class design.
    I would agree that this statement seems to sum up the problem here.

    dynamic_cast<> is not necessarily all that efficient at run time)
    Definitely an understatement. I used to have a link to a website that compared all the casts and by far dynamic was the slowest. Of course this may depend on compiler implementation and so forth but in general dynamic_cast is quite slow. You really should never have to query an object for it's C++ type. This points to a flaw in the design of the system.
    Last edited by VirtualAce; 04-23-2010 at 11:33 PM.

Popular pages Recent additions subscribe to a feed

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21