C++ ambiguous overload

[sweenish]

I am working on a class project, so this an error of my own making.

I am working on a basic mad-lib maker. The user is presented a menu which contains: two example mad libs, a create your own option, an open from file option, and quit.

After an example is run through (the user actually fills in the requested words), another menu is presented asking if they want the story printed to the screen or to a file. This sub menu is its own function, and it also acts on the user's choice. This function is where I'm running into issues elsewhere.

So, when a user creates a story (enters the prompts they want and the story chunks), I give them the option to run the story right now, or save the template to a file. That sub-menu is its own function as well. The issue I'm having is that if the user chooses to run the story now, I want to able to present the sub-menu from above (print to screen or to file) for consistency. But I can't call that sub menu function from within my current sub menu.

Here's my main.cpp. The interaction I want is to call sub_for_ex() from within sub_for_create()

 * main.cpp

#include <iostream>
#include <string>
#include <vector>
#include "main.hpp"
#include "Story.hpp"
#include "space_story.hpp"
#include "medievalstory.hpp"
#include "CreateStory.hpp"
#include "utils.hpp"

using namespace std;

int main (void) {
    bool flag = true;
    int choice;
    string tale;
    Story* sStory = new SpaceStory;
    Story* mStory = new MedievalStory;
    Story* cStory = new CreateStory;

    cout << "Welcome to Mad Lib Maker!" << endl;
    do {
        // Main menu is handled in its own function
        choice = show_main_menu();

        // The user's choice is handled here
        switch (choice) {
        case 1:
            sStory->get_story();
            sub_for_ex_menu(sStory);
            break;
        case 2:
            mStory->get_story();
            sub_for_ex_menu(mStory);
            break;
        case 3:
            cStory->get_story();
            sub_for_create(cStory);
            break;
        case 4:
        case 5:
            flag = false;
            break;
        }
    } while (flag == true);

    delete sStory;
    delete mStory;
    delete cStory;

    return 0;
}


int show_main_menu()
{
    int choice;

    // Menu persists until a valid choice is made
    do {
        cout << endl << endl;
        cout << "***Main menu***" << endl;
        cout << "  1. Example: Space Story" << endl;
        cout << "  2. Example: Medieval Story" << endl;
        cout << "  3. Create Your Own!" << endl;
        cout << "  4. Open a Custom Story" << endl;
        cout << "  5. Quit" << endl;
        choice = get_int("Please enter the number of your choice: ");

        // get_int makes sure a number is entered, this makes sure an appropriate number is entered
        if (choice < 1 || choice > 5) {
            cout << endl << "Please enter a number from 1 to 5." << endl;
        }
    } while (choice < 1 || choice > 5);
    cin.ignore();    // Clear carriage return for getline()

    return choice;
}


void sub_for_ex_menu(Story* &s)
{
    int choice;
    string sto;
    string fname;

    // Persistent sub-menu after completing an example mad lib
    do {
        cout << "Would you like the story printed to the screen, or to a file?" << endl;
        cout << "  1. Print to screen" << endl;
        cout << "  2. Save to file" << endl;
        choice = get_int("Please enter the number of your choice: ");

        if (choice < 1 || choice > 2) {
            cout << "Please enter either 1 or 2." << endl;
        }
    } while (choice < 1 || choice > 2);
    cin.ignore();    // Clear carriage return for getline()

    sto = s->make_story();
    if (choice == 1) {
        // Print story to screen was chosen
        s->print_story(sto);
    }
    else {
        // Completed story is saved to a file
        cout << "Please enter a file name (NO spaces): ";
        getline(cin, fname);
        fname = clean_spaces(fname);
        s->save_story(fname, sto);
    }
}


void sub_for_create(Story* &t)
{
    int choice;
    string fname, sto;

    // Persistent menu run after a Story is created
    do {
        cout << endl << "Would you like to run the story now, or save it for later?" << endl;
        cout << "  1. Run the story now" << endl;
        cout << "  2. Save it for later" << endl;
        choice = get_int("Please enter the number of your choice: ");

        if (choice < 1 || choice > 2) {
            cout << "Please select either 1 or 2." << endl;
        }
    } while (choice < 1 || choice > 2);
    cin.ignore();    // Clear carriage return for getline()

    if (choice == 1) {
        // The words are got and the story is printed to the screen
        cout << endl << endl;
        t->Story::get_story();
        sto = t->make_story();
        /*** WANT TO CALL SUB_FOR_EX HERE ***/
        t->print_story(sto);   // Don't want to use this
    }
    else {
        // A template file is saved that will allow the story to be run later
        cout << "Please enter a file name (NO spaces): ";
        getline(cin, fname);
        fname = clean_spaces(fname);
        t->save_story(fname);
    }
}

If I try to include that call, passing just t, I get a compile error that the overloaded function is ambiguous, and I haven't found a way around it.

As it stands, if the user wants to immediately run a story they created, it will just print to screen. I can get around that by replicating code, but the whole point of this course (Intro to OOP) is that replicating code shouldn't be a thing.

I think main.cpp is enough to figure this out, as my issue isn't with my classes or objects directly, but if you guys want to see more of the project, I'll happily upload a tar of my Eclipse project.

Always appreciate the assistance. You guys have helped me a ton in the past.

 

[Ken g6]

Does the compiler give you a list of the candidates for the ambiguous function, like this site shows? http://stackoverflow.com/questions/4672152/call-of-overloaded-function-is-ambiguous

 

[Ancalagon44]

My C++ is a little rusty, but why are your functions declared like this:
void sub_for_ex_menu(Story* &s)

Surely that would mean you are passing a pointer to a reference?

I thought it should be:
void sub_for_ex_menu(Story* s)

Try making that change to both functions and see what happens. My hunch is that you won't be able to call one from the other - try calling sub_for_create from within sub_for_ex and it won't work either.

My guess is that it is because you are changing the pointer to a reference by passing it in. So, it isn't the same type as it is in the main function, which is why you can't then pass it to the other function.

EDIT:
Also I didn't know this was valid C++:

Story* sStory = new SpaceStory;
I thought it would have to be:

Story* sStory = new SpaceStory();

I don't know if you have to, but when I delete memory (when I used to program using C++), I always set the pointer to 0 afterwards.

Like so:
if(pointer)
{
delete pointer;
pointer = 0;
}

 

[sweenish]

Does the compiler give you a list of the candidates for the ambiguous function, like this site shows? http://stackoverflow.com/questions/4672152/call-of-overloaded-function-is-ambiguous

I went to add the function call back in, and it compiled with no errors. It's possible I didn't fully re-factor earlier or some other oversight on my part. When I ran it, it also showed the menu (sub_for_create successfully called sub_for_ex), but it's stuck somewhere else now. After entering a choice, it freezes. I think I can figure that one out on my own. I'll drop some couts to see where it's hanging.

It's possible it still won't work.

My C++ is a little rusty, but why are your functions declared like this:
void sub_for_ex_menu(Story* &s)

Surely that would mean you are passing a pointer to a reference?

I'm passing a reference of a pointer. So, the address of a specific pointer, which is pointing to the object I want to modify. Otherwise I would be messing with a pointer to a pointer, wouldn't I? I would rather modify my object as directly as possible.

At least that's my understanding of what it's doing.

 

[purbeast0]

Does the compiler give you a list of the candidates for the ambiguous function, like this site shows? http://stackoverflow.com/questions/4672152/call-of-overloaded-function-is-ambiguous

yeah this is what i would expect as well, it should tell you what the different options are.

then just figure out which one you need and cast it to that by putting "(<type>)" infront of it, without the quotes, and where <type> is the actual type you have to cast it to.

also, please use camelCase instead of underscore_notation for variable names

 

[sweenish]

Noted on naming convention. I've been on this project non-stop for the last week; I think I just got frazzled and mixed stuff up.

I've also fixed the freeze I mentioned earlier. I was calling a class function twice, and the second time it was being called, it wasn't being passed anything since the parameters were destroyed (I pop the values out of my vectors until they're empty). I've added a check and error message so I can catch it faster if it happens again.

EDIT: Looking at my main.cpp, I don't see any variables using '_'. Functions, yes. I'll still comb through my other classes. The only one I can't change completely at the professor's request are private member variables. Those are always m_blah. I can at least camelCase every other part of the variable name.

EDITx2: I will make sure to include actual error text the next I ask for help here. Sorry for not including it initially.

 

[purbeast0]

oh yeah, i meant variable and method/function names camel case them. just a personal thing. i actually hate reading c/c++ code in general because all of the stupid outdated naming "conventions" that seem to be stuck around from the 90's.

 

[Ancalagon44]

I'm passing a reference of a pointer. So, the address of a specific pointer, which is pointing to the object I want to modify. Otherwise I would be messing with a pointer to a pointer, wouldn't I? I would rather modify my object as directly as possible.

At least that's my understanding of what it's doing.

Try what I suggested and let me know if it works.

No, having a look at my old code, I don't think that what you are writing, is doing what you want it to do.

Put it this way - when you pass a pointer, you can either modify the pointer itself or the memory address that it points to.

So, if you did something like the following:
Story* myStory = new Story();
call_function(myStory);

Where call_function accepts a Story* paramater.

Inside call_function, you could do the following:
myStory = 0;

Which is modifying the pointer but not the memory it points to. In fact, this will create a memory leak unless you have another pointer to the memory previously pointed to by myStory.

If you want to interact with that memory, you would do the following:
myStory->method();

You deference the pointer to interact with it.

Referencing is usually only used for objects on the stack, not the heap.

Two examples:

void CampaignState::interpretResult(BattleResult* result)
{
mBattleResult = result;
mDisplayBattleResults = true;
PlayerResult* pResult = result->getPlayerResult(mPlayerProfile);

if(pResult)
{
std::deque<PlayerResult*> players = result->getPlayerResults();
///code deleted for brevity

You would call that by doing:

mBattleResultInterpreter = new BattleResultInterpreter();
mBattleResultInterpreter->interpretResult(mBattleResult);

(Note that CampaignState inherits from BattleResultInterpreter)

For references, you would do something like the following:

void PlayerPreBattleState::setAIScriptName(const std::string& script)
{
mAIScriptName = script;
}

Which you would call by doing the following:

enemy1PState->setAIScriptName("aiscript.lua");

These code snippets are examples from a C++ game I worked on a while back.

 

[sweenish]

I get that if I null the pointer without deleting the dynamically allocated object, I'll have a leak. That's simply not happening in my code.

I'm passing a variable of type "pointer to an object." The ampersand has me passing the address of a pointer to an object. Stored at that address (the one I pass with the &) is the address of my dynamically allocated object. If pointers are already passed by reference and not by value, then I guess it's just redundant code.

Story is my base class, so I declare my derived objects as Story* because the base class provides a common interface for all my objects. At no point do I or my code intend to directly modify the Story*, just the object being pointed to.

That's my logic, anyway. And it at least compiles without errors. I realize that's not a very good metric for good code, but I think it still counts for something.

I'll just drop a link to the project.
https://drive.google.com/open?id=0B_1IHjrPyyXDTmtnOTVuTXVTNDg

I imagine you guys will find some glaring issues and ways to quickly optimize it further; that's just experience I don't have yet. But right now, it's at least complete, compiles clean, and largely works as intended. This version doesn't have a few extra messages to make it clear to the user what's happening.

EDIT: I removed the ampersands, and it still compiles fine and behaves well as far as I can see. So I'll just leave them out.

 

[SunnyD]

This will undoubtedly come off sounding a little condescending, but I assure you it isn't meant that way.

It sounds to me like you need to learn/understand better the difference between passing by reference and passing by value better and also the difference, with respect to C++, between objects and pointers to objects. At least from what I'm seeing, that's where it appears you're getting confused.

Edit in progress...

With C++ you're going to define a class object which you can instantiate in one of two ways:

#1. On the stack
#2. On the heap

Let's start with #1...

When you create your object (or even a simple type), you can instantiate it on the stack by simply declaring an instance of its type like so:

MyObject o;
o.myInt = 42;

Voila. Simple. You've got that down cold.

When you're instantiating on the heap, you have to use one of many different memory allocators to allocate an instance on the heap and grab a pointer reference to it. Then you need to stick that pointer reference somewhere. (Don't confuse my using the word "reference" here with "passing by reference", it's sort of the same but not for this context):

MyObject * pointerToMyObject = (MyObject*)new MyObject();
pointerToMyObject->myInt = 42;
...
delete pointerToMyObject;

Another something you're probably pretty familiar with. But what's the difference between the two different instances? Well, as one mentioned, the former was created on the application's local (or global) stack and it's lifetime is managed by the application itself. You use the dot referencing operator to access its members (if it's not a simple type) because you're accessing the object itself.

The latter was created on the heap by the memory manager, its lifetime specified by the user. You have a pointer to the object, rather than the object itself and you access its members with the deferencing arrow operator. All the rules that come into play dealing with pointers come into play here.

Now where does passing-by-reference and passing-by-value come into play here? This is where a lot of programmers have a love-hate relationship with C/C++.

By default, C++ passes by value. So given a function:

SomeFunction(int aValue)

SomeFunction will make a copy of whatever is populated into aValue on the stack and the copy into the code for SomeFunction. Any modifications to aValue within the code for SomeFunction will never affect any original variable tied to the call into SomeFunction. But you knew this. This leads us to pointers...

SomeFunctionThatTakesAPointer(int * aPointer)

SomeFunctionThatTakesAPointer takes a parameter that is a pointer. Does that mean it's by-reference? Nope. Yes, the pointer is a reference to something, by the parameter itself is still passed by-value. Here's why:

...
int myInt = 42;
// I need to do something special with myInt
SomeFunctionThatTakesAPointer(&myInt);

And I'm sure you know that &myInt is simply taking the address of myInt (a pointer), aka: referencing it and passing it along. So in a sense yes, it got passed by reference. But What really happened is the address getting copied to a temporary variable on the stack parameter, and within SomeFunctionThatTakesAPointer you will still need to manually dereference it. So you're still just passing a pointer by value, not a reference.

So what is passing by reference? I think at this point you should be able to put it together.

SomeFunctionByReference(MyRefObject &ob)
{
  if(ob.myInt == 42)
  {
    // Do something really special
  }
}

With passing-by-reference, you deal in just the actual stack based objects. All of the pointer-fu is hidden from you. It's still there, but the compiler and runtime handle it all transparently for you, letting you think everything is just one seamless piece of code. Passing by reference gives you the benefits of passing a pointer-to-an-object without the mess of manually having to dereference the object every time. It also ensures the lifetime and scope of the object is maintained because it won't let the programmer errantly delete the object when they shouldn't. How does it work? It's simple:

...
MyRefObject ob;
ob.myInt = 42;
ob.methodDoesSomethingReallyImportant();
// Now I need to hand ob to someone else because they need it really badly
SomeFunctionByReference(ob); // The by-reference type automatically handles the behind the scenes stuff

That's it. No pointers were harmed in the making of this reference object.

There's things to be careful of of course; const correctness, copy operators, etc. But those are things you'll need to learn about. This is simply about reference objects.

Lastly...

oh yeah, i meant variable and method/function names camel case them. just a personal thing. i actually hate reading c/c++ code in general because all of the stupid outdated naming "conventions" that seem to be stuck around from the 90's.

Ignore this. Use whatever you're comfortable with until you actually get into a working environment, because your working environment will dictate your coding standards. And trust me, every employer... even every team within every employer... has completely different coding standards.

 

[Ancalagon44]

SunnyD explained what I was trying to explain, but much better. Listen to him.

Do some reading up about pointers and references, and the differences between the two, and when you would use one or the other.

 

[purbeast0]

Ignore this. Use whatever you're comfortable with until you actually get into a working environment, because your working environment will dictate your coding standards. And trust me, every employer... even every team within every employer... has completely different coding standards.

obviously there are coding standards that are different everywhere, but the shorthand archaic way of naming conventions with abreviated words simply isn't necessary anymore. it was necessary back in the day because memory and resources were so limited, so you wanted to use as little as necessary.

the underscores is just personal preference, i can't stand it and i find it harder to read than camelCase. it's also harder to type.

when i code my goal is to code without the need for comments explaining what each function/variable is doing. my goal is to make it readable so comments aren't necessary for those things, and that comments are only necessary to explain the business rules and why things are happening, not what is happening. what is happening should be obvious from the code.

in general, his code is pretty readable, i was just nitpcking _ since i don't like them. but these are examples that are simply bad practice imo.

    Story* sStory = new SpaceStory;
    Story* mStory = new MedievalStory;
    Story* cStory = new CreateStory;
    string sto;
    string fname;

why not just make it:

    Story* spaceStory = new SpaceStory;
    Story* medievalStory = new MedievalStory;
    Story* createStory = new CreateStory;
    string story;
    string fileName;

if i saw 'sStory' or 'cStory' down later in the code, i would have no clue what those means. most programmers can type pretty fast - type words out.

also not a fan of the name of the class "CreateStory" as I think putting verbs into class names is just bad practice. objects should be nouns, functions should be verbs.

 

[SunnyD]

the underscores is just personal preference, i can't stand it and i find it harder to read than camelCase. it's also harder to type.

...

in general, his code is pretty readable, i was just nitpcking _ since i don't like them. but these are examples that are simply bad practice imo.

It's all good. I completely understand why you said it even before you posted this, and I wasn't trying to be a snot about even your post. I personally "grew up" in the Microsoft era of m_Variable and szStrings, so old habits die hard. My coworkers hate me for it when they slip through. Things like ReSharper make enforcing coding standards for the old guys like me a bit less of a tedium. But my point for the new programmers like sweenish (and by extension to you when you're trying to explain stuff to the new guys) is to not worry about the static minutia like coding standards. They're meaningless in the grand scheme of things, especially when the person really wants to learn the fundamentals of something programming related.

I guarantee you that 10 years from now even our current coding standards will be outdated anyway and people will be scoffing at what we use now.

 

[Ancalagon44]

I don't like underscores in variable names, but when I was working in C++, I did make use of prefixes to denote the scope of a variable. m for member, etc. I found that quite useful.

I bought Effective C++ Third Edition, which I found a very good read on C++. It mentioned a lot of things that I used to improve my C++, just would not have known otherwise.