Runtime error, Unknown problem. (Homework)

Hi everyone.

Im new to this forum and hoping for a little help. I have done some C programming before, and now am doing a C++ course in my second undergrad year, but having a little trouble with this one program. Id appreciate any help that you could offer me as I have tried everything that I can possibly think of to try and get it working.

So, what the program does:
Set up a class describing a planet containing x,y,z and an id.
Create an array to point to these planets (10 of).
Use a function to take co-ordinates from a text file, add it to a created class and then pass it back into the array.
Display the co-ordinates on screen to prove it works.
Work out the total distance between all of these planets.

So, whats wrong:
The program compiles correctly, works for the first three lines of the text file, then promptly tells me there is a problem, quitting the program. I think, but cannot be sure, that the problem occurs when the generate_planet function is called and creates a new planet class.

The code:

Code:

---

int main(void)
{
    int i = 0;
    float total_distance = 0.0;
    ifstream fin;
    fin.open("route.txt");
    planet* the_planets[10];
    for( i = 0 ; i < 2 ; i++ )
    {
        *the_planets[i] = generate_planet(fin);
        report_planet(*the_planets[i]);
    }
    for( i = 0 ; i <= 2 ; i++)
    {
        total_distance += distance(&the_planets[i], &the_planets[i+1]);
    }

    /*
    planet mars, earth;
    get_planet(&mars);
    get_planet(&earth);
    cout << "distance is " << distance(&mars, &earth);
    return 0; */
}


void report_planet(planet report)
{
    cout << "Co-ordinates for " << report.id << endl;
    cout << "x : " << report.x << endl;
    cout << "y : " << report.y << endl;
    cout << "z : " << report.z << endl;
}

void get_planet(planet *get)
{
    cout << "Please enter planet ID\n";
    cin >> get->id;
    cout << "Please enter x co-ordinate\n";
    cin >> get->x;
    cout << "Please enter y co-ordinate\n";
    cin >> get->y;
    cout << "Please enter z co-ordinate\n";
    cin >> get->z;
}

float distance(planet *planet1, planet *planet2)
{
    float distance=0.0;
    distance = sqrt( pow((planet1->x - planet2->x),2) + pow((planet1->y - planet2->y),2) + pow((planet1->z - planet2->z),2) );
    return distance;
}

planet &generate_planet( ifstream &fin )
{

    planet *newplanet = new planet;
    fin >> newplanet->id >> newplanet->x >> newplanet->y >> newplanet->z;
    return *newplanet;
}

---

The text file:

Code:

---

1 9 8 6
2 6 8 2
3 7 8 1
4 1 1 2
5 21 11 1
6 7 11 0
7 11 18 1
8 5 3 1
9 9 9 17
10 8 1 1

---

Thanks for any help, I really am at a loss to where the problem occurs in this program and I appreciate any comments about any part of the code, but in particular the section that is not working.

Code:

---

    planet* the_planets[10];
    for( i = 0 ; i < 2 ; i++ )
    {
        *the_planets[i] = generate_planet(fin);
        report_planet(*the_planets[i]);
    }

---

You're declaring an array of pointers, but you haven't directed those pointers at anything valid. They contain garbage. Thus when you dereference them, you're trying to write planet objects to random portions of memory.

I suggest you drop the pointer usage entirely unless you have a good reason for it. Your usage shows you don't really understand how they're supposed to work anyway.

They have to be pointers stored in an array according to the guidelines... however much better other methods could be/are. The pointers are then assigned an address that comes back from the generate_planet(fin) function.

That isn't anywhere in what you posted. Perhaps you should show us the exact requirements.

Quote:

---

Originally Posted by farso

They have to be pointers stored in an array according to the guidelines... however much better other methods could be/are. The pointers are then assigned an address that comes back from the generate_planet(fin) function.

---

Quote:

---

Originally Posted by Lindley

Code:

---

    planet* the_planets[10];
    for( i = 0 ; i < 2 ; i++ )
    {
        *the_planets[i] = generate_planet(fin);
        report_planet(*the_planets[i]);
    }

---

You're declaring an array of pointers, but you haven't directed those pointers at anything valid. They contain garbage. Thus when you dereference them, you're trying to write planet objects to random portions of memory.

---

Farso, Lindley is right. What you are doing is assigning the CONTENT, and NOT the address. Even if the content is dynamically allocated, it will still crash. You should try this instead:

Code:

---

    planet* the_planets[10] = {0};
    for( i = 0 ; i < 2 ; i++ )
    {
        the_planets[i] = &generate_planet(fin);
        report_planet(*the_planets[i]);
    }

---

This should work.

Look at this:

Code:

---

int *p1;
int *p2;
int a1 = 5;
int a2 = 5;

p1=&a1;  //1
*p2=a2;  //2

---

It is IMPERATIVE you understand the difference between line 1 and 2, and that you understand why line 2 will create a runtime error somewhere along the program.

Sorry, Perhaps I was not clear in my first post about the function of the program.

class planet - store an int id, and floats z, y and z.
function report_planet - takes the argument identifying a planet and returns its details.
function get_details - takes an argument of an existing planet and changes the details.
function generate_planet - takes stream argument connected to file, creates a new instance of planet and read the details from the attached stream. The function returns the newly created planet using a planet pointer.
function distance - calculates the distance between two planets.

main - connects stream, defines a planet pointer array of 10 places, points each of the pointers to the address of the newly created planet using generate_planet function. Reports back each of the details of the entered planets, then works out the total distance between all of them.

I hope I have been a little clearer hear as to what I am trying to achieve. I didnt think I was too far away from the actual goal as it works for the first three planets. Please dont think Im just here for a free answer, I would like to learn what I am doing wrong.

Thanks for your patience

Quote:

---

Originally Posted by monarch_dodra

Farso, Lindley is right. What you are doing is assigning the CONTENT, and NOT the address. Even if the content is dynamically allocated, it will still crash. You should try this instead:

Code:

---

    planet* the_planets[10] = {0};
    for( i = 0 ; i < 2 ; i++ )
    {
        the_planets[i] = &generate_planet(fin);
        report_planet(*the_planets[i]);
    }

---

This should work.

Look at this:

Code:

---

int *p1;
int *p2;
int a1 = 5;
int a2 = 5;

p1=&a1;  //1
*p2=a2;  //2

---

It is IMPERATIVE you understand the difference between line 1 and 2, and that you understand why line 2 will create a runtime error somewhere along the program.

---

Thank you so much. So what I was doing was creating a pointer which was pointing at the address of another pointer?

What you *were* doing was trying to forget you were using pointers at all and copying the data they pointed to around. What you *should* be done is copying the pointer itself instead.

It's also vital for you to delete the allocated planet objects when you're done with them.

I think I understand what I did now, seems quite stupid when looking at it. I think its the hardest concept of the language to implement correctly. Like grammar in a language. I think what I was actually thinking was assign the pointer then pass the pointer around? I understand correctly in the line by line type of code, just when it gets to passing between functions that becomes more complicated.

Does the compiler and program not automatically delete these bits of data after it is done with them?

Quote:

---

Originally Posted by farso

Does the compiler and program not automatically delete these bits of data after it is done with them?

---

When the program exits, the OS is smart enough to reclaim everything that the program was using. But for the duration of the program, no. C++ isn't a garbage-collected language. You need to clean up after yourself. Every "new" needs a corresponding "delete". There has to be a one-to-one match there. (The exception being if you explicitly pass responsibility for deleting an object to another object---for instance, a smart pointer.)

And you're correct that pointers are one of of the less intuitive aspects of the language. That's why the STL tries to abstract things so that you don't have to deal with pointers directly very often. You can always tell C programmers from C++ programmers---the former are much heavier in their use of pointers.

So long as all your planet objects are being held by pointers anyway, you should modify your functions to take planet pointers rather than planet objects. If the function doesn't modify its argument, you should declare the parameter to be a const planet*. This won't affect correctness in this toy example, but if you were working with objects which were uncopyable (and thus had to be stored by pointer this way), that's how you'd do it.