Thread: An issue with Switch (Scope?)

  1. #1
    Registered User
    Join Date
    May 2008
    Posts
    110

    An issue with Switch (Scope?)

    Hello,

    I have a function similar to this for character construction (types omitted).

    The switch statement does not work. I though it is an issue of score, but setting curly brackets inside each case does not help. The actual switch statement is quite long, because there are many admissible combinations of weapons (and more than two weapons are allowed).

    Can you please help with an advice? Thanks!

    Code:
    CowboyPtr ConstructCowboy(std::string name, const int loadoutcode)
    {
    	CowboyPtr cowboy = CowboyPtr(new Cowboy(name));
    	switch (loadoutcode) {
    		default:
    		WeaponPtr leftweapon = WeaponPtr(new Colt());
    		WeaponPtr rightweapon = WeaponPtr(new Whip());
    		break;
    		case 1:
    		WeaponPtr leftweapon = WeaponPtr(new Colt());
    		WeaponPtr rightweapon = WeaponPtr(new Rifle());
    		break;
    	}
    	cowboy->Loadout.insert(WeaponPair(leftweapon->Range, leftweapon));
    	cowboy->Loadout.insert(WeaponPair(rightweapon->Range, rightweapon));
    	return cowboy;
    }

  2. #2
    Registered User
    Join Date
    May 2012
    Location
    Arizona, USA
    Posts
    657
    Each case creates two variables, leftweapon and rightweapon, and they both go out of scope outside the switch statement. Define them before the switch statement and only assign to them inside the switch statement.

  3. #3
    Registered User
    Join Date
    May 2008
    Posts
    110
    Ah..get it! So I need:

    Code:
    WeaponPtr leftweapon = nullptr;
    WeaponPtr rightweapon = nullptr;

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,268
    A few stylistic issues to address:
    [list][*]You didn't provide the code that shows what are the types involved, but I'm going to guess that CowboyPtr and WeaponPtr are just typedefs for Cowboy* and Weapon* respectively. If so, get rid of these typedefs: they are useful when you are naming your own smart pointer type, but otherwise they just obscure the pointer type.[*]Avoid directly using the new operator where feasible. In practice, this means that if you can use a container or std::make_unique or std::make_shared, you should do so.[*]Use auto instead of repeating type names verbosely.[/list
    Combined, you might end up with something like this:
    Code:
    std::unique_ptr<Cowboy> ConstructCowboy(const std::string& name, int loadoutcode)
    {
        using std::make_unique;
    
        auto cowboy = make_unique<Cowboy>(name);
        std::unique_ptr<Weapon> leftweapon;
        std::unique_ptr<Weapon> rightweapon;
        switch (loadoutcode) {
        case 1:
            leftweapon = make_unique<Colt>();
            rightweapon = make_unique<Rifle>();
            break;
        default:
            leftweapon = make_unique<Colt>();
            rightweapon = make_unique<Whip>();
        }
        cowboy->Loadout.insert(WeaponPair(leftweapon->Range, leftweapon));
        cowboy->Loadout.insert(WeaponPair(rightweapon->Range, rightweapon));
        return cowboy;
    }
    It may not compile due to the Loadout related code (you may need to use std::move or release the unique_ptr), but then you have not shown the relevant code behind WeaponPair and Loadout so I leave those for you to fix.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    May 2008
    Posts
    110
    Thank you very much! I will rethink the style (still very new to cpp, and it shows). Over the past few months, I have learnt so much from all of you in this forum and I greatly appreacite it.

    I have a follow up question (sorry, still in the old style):

    I need to instantiate a player by calling a function that returns a typed (shared) pointer, which can either be typed KnightPtr or CowboyPtr.

    The pointer goes out of scope when leaving the if-statement. How can I preserve the pointer by defining it outside of the if-statment if I don't know which type will come out of the if-statement?

    I guess I need some generic pointer to by cast inside the if-statement. How best to do this?

    Code:
    if (A_type == "knight") {
    	KnightPtr A = ConstructKnight("\nKnight A", A_loadout);
    } else {
    	CowboyPtr A = ConstructCowboy("\nCowboy A", A_loadout);
    }
    
    PrintPlayer(A);
    For example somethink like:
    Code:
    PlayerPtr A = nullptr;
    
    if (A_type == "knight") {
    	// cast A to KnightPtr
    	A = ConstructKnight("\nKnight A", A_loadout);
    } else {
    	// cast A to CowboyPtr
    	A = ConstructCowboy("\nCowboy A", A_loadout);
    }
    
    PrintPlayer(A);
    More info: classes Knight and Cowboy inherid from Player, but are not polymorphic to it (class Player is not vitrual). The function PrintPlayer (despite its name) cannot be applied to Player but only to Knight or Cowboy, because Player does not hold all the data.
    Last edited by serge; 10-17-2019 at 08:56 AM.

  6. #6
    Registered User
    Join Date
    May 2008
    Posts
    110
    Quote Originally Posted by laserlight View Post
    A few stylistic issues to address:
    [list][*]You didn't provide the code that shows what are the types involved, but I'm going to guess that CowboyPtr and WeaponPtr are just typedefs for Cowboy* and Weapon* respectively. If so, get rid of these typedefs: they are useful when you are naming your own smart pointer type, but otherwise they just obscure the pointer type.[*]Avoid directly using the new operator where feasible. In practice, this means that if you can use a container or std::make_unique or std::make_shared, you should do so.[*]Use auto instead of repeating type names verbosely.[/list
    Combined, you might end up with something like this:
    Code:
    std::unique_ptr<Cowboy> ConstructCowboy(const std::string& name, int loadoutcode)
    {
        using std::make_unique;
    
        auto cowboy = make_unique<Cowboy>(name);
        std::unique_ptr<Weapon> leftweapon;
        std::unique_ptr<Weapon> rightweapon;
        switch (loadoutcode) {
        case 1:
            leftweapon = make_unique<Colt>();
            rightweapon = make_unique<Rifle>();
            break;
        default:
            leftweapon = make_unique<Colt>();
            rightweapon = make_unique<Whip>();
        }
        cowboy->Loadout.insert(WeaponPair(leftweapon->Range, leftweapon));
        cowboy->Loadout.insert(WeaponPair(rightweapon->Range, rightweapon));
        return cowboy;
    }
    It may not compile due to the Loadout related code (you may need to use std::move or release the unique_ptr), but then you have not shown the relevant code behind WeaponPair and Loadout so I leave those for you to fix.
    Another question that came to my mind is why does inserting e.g. weapon in loadout increase the counter of shared pointers (I am using shared)? My types are:

    Code:
    typedef std::shared_ptr<Weapon> WeaponPtr;
    typedef std::pair<int, WeaponPtr> WeaponPair;

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,268
    Quote Originally Posted by serge
    More info: classes Knight and Cowboy inherid from Player, but are not polymorphic to it (class Player is not vitrual). The function PrintPlayer (despite its name) cannot be applied to Player but only to Knight or Cowboy, because Player does not hold all the data.
    I think that this is the fundamental problem: you should make Player a polymorphic base class, defining a common interface that Knight and Cowboy can both implement, and which will be used by the PrintPlayer function.

    If you really cannot do this, then it implies that Knight and Cowboy aren't really Player subclasses except for the sole purpose of inheriting implementation. Therefore, PrintPlayer no longer makes sense: you should have PrintKnight and PrintCowboy instead, with entirely separate code paths for creating one of them and printing it.

    Quote Originally Posted by serge
    Another question that came to my mind is why does inserting e.g. weapon in loadout increase the counter of shared pointers (I am using shared)?
    You're copying the shared pointer, so it has to increase the reference count. After all, if the original pointer is destroyed, you still want the pointer in the pair to be around.
    Last edited by laserlight; 10-17-2019 at 04:05 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Registered User
    Join Date
    May 2008
    Posts
    110
    Quote Originally Posted by laserlight View Post
    I think that this is the fundamental problem: you should make Player a polymorphic base class, defining a common interface that Knight and Cowboy can both implement, and which will be used by the PrintPlayer function.
    But still it should be possible to recast pointers of unrelated classes, I just can' wrap my head around how:

    Something like this should doable:

    Code:
    auto A = nullptr;
    if (true) {
      MyType B = foo(); // foo() returns a shared pointer
      // copy B to A with type conversion, A does not go out of scope
    }

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,268
    Quote Originally Posted by serge
    But still it should be possible to recast pointers of unrelated classes, I just can' wrap my head around how:
    Doing that reinterprets an object of one type as being of another type via pointers, and isn't what you want here. What you're probably thinking of is a pointer to void, but that is not type safe, and anyway that isn't going to work with a shared_ptr. (You could get a pointer that could be converted to void* from a shared_ptr, but once the shared_ptr goes out of scope, that pointer to void becomes invalid.)

    Even if you switched from a shared_ptr so as to use a pointer to void, you have other problems: you could pass the pointer to void to PrintPlayer, but PrintPlayer would have no clue as to what is the underlying type, so you would need some way to specify it, and you'll be hardcoding PrintPlayer (and every other "generic" Player function) for each possible subclass of Player, whereas if you used polymorphism, you could add new Player subclasses without having to update PrintPlayer.

    Why are you trying so hard to avoid better design?
    Last edited by laserlight; 10-18-2019 at 11:32 AM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Switch case variable scope
    By VirtualAce in forum C# Programming
    Replies: 7
    Last Post: 02-01-2014, 08:45 AM
  2. Scope issue with header file used as interface
    By Rath in forum C Programming
    Replies: 4
    Last Post: 02-02-2012, 05:04 PM
  3. Case Switch issue
    By pc_doctor in forum C Programming
    Replies: 8
    Last Post: 12-06-2008, 06:24 AM
  4. Basic scope issue
    By Dondrei in forum C++ Programming
    Replies: 4
    Last Post: 07-06-2008, 06:36 AM
  5. nested switch issue
    By fsu_altek in forum C Programming
    Replies: 3
    Last Post: 02-15-2006, 10:29 PM

Tags for this Thread