0

When I attempt to run this code it crashes. There are no error messages. When the program compiles and runs, it just displays the windows 7 message, "this program has stopped working.":

void readGameFile(string ** entries, int * num_entries, string ** story, int * num_lines)
{
    ifstream madlib("madlibs1.txt");
    string line;
    getline(madlib, line);
    *num_entries=stoi(line);
    *entries=new string [*num_entries];
    for (int i=0; i<*num_entries; i++)
    {
        getline(madlib,*entries[i]);
    }

I did a few tests, and it seems to assign entries[0] a value, and then crashes when attempting to assign entries[1] a value. I am forced to use this function name, with those function parameters and parameter types specifically. I also may not use malloc, vector or other answers I've seen.

Improve this question
9
  • Can you check using is_open() if the file has been opened ? Commented Oct 17, 2013 at 7:24
  • I have checked in various ways that the file opens. It is capable of reading things from the file. It seems to have to do with the way I am dynamically allocating. Now it's giving me a debug error and says "abort has been called." Commented Oct 17, 2013 at 7:34
  • 2
    You would likely be surprised how much cleaner this is using std::vector<std::string> objects. That aside, exactly none of your io operations are being checked for success. Commented Oct 17, 2013 at 8:02
  • 1
    This is a dev's world...but it wouldn't be nothing, witouth a try or a catch (both)... Commented Oct 17, 2013 at 8:04
  • 1
    @Ameoo agreed. there are several things that could toss exceptions in this, std::stoi being one of them. And without a try-catch block handler, the program is as good as dead shortly thereafter. Commented Oct 17, 2013 at 8:08

1 Answer 1

Reset to default
1

I think the issue is one of precedence: you almost certainly want:

getline( madlib, (*entries)[i]) );

Otherwise, you're indexing from the string**, then dereferencing: *(entries[i]).

You also want to check the results of getline, possibly in the loop:

for ( int i = 0; madlib && i != *num_entries; ++ i )...

as well as before the std::stoi.

And finally: I don't know why you are forced to use this function signature. It is horrible C++, and you should never write anything like this. Logically, std::vector<string> would be a better solution, but even without it: your function has 4 out parameters. This would be better handled by returning a struct. And failing that, out parameters in C++ are usually implemented by non-const reference, not by a pointer. While there are arguments for using the pointer in some cases, when it results in a pointer to a pointer, it's evil. If nothing else:

bool        //  Because we have to indicate whether it succeed or failed
readGameFile( std::string* &entries, int &num_entries, std::string* &story, int &num_lines )
//  ...

(This actually looks more like it should be constructor, however, of a class with two data elements, entries and story.)

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

+1 I was noticing the dereference precedence as well, but for the life of me couldn't come up with a presentation that made sense in my mind. I really like yours with the two examples.
It's all fixed now. I appreciate it. Thank you very much. To answer your question, this is part of a college assignment. Thus the silly restrictions...
@user2889285 If this is an assignment, I have my doubts about the competence of the person who gave it.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.