Help with structures practice

[treebeard]

The practice problem asks me to create an array of spaceship structures and a program that will continuously update their position as long as they are all within a 1024x728 grid. this is my code

#include <iostream>
#include <cstdlib>
#include <ctime>


using namespace std;
int num;


struct spaceShip
{
    int x;
    int y;
};


int randRange (int low, int high);
spaceShip update (spaceShip ship);
bool inbounds (spaceShip my_ships[],int num);


int main ()
{
    cout << "How many space ships would you like to simulate?\n";
    cin >> num;
    
    spaceShip my_ships[num];


    cout << "Generating random starting coordinates..." << endl;
    for (int i = 0; i < num; i++)
    {
        my_ships[i].x = randRange(0,1024);
        my_ships[i].y = randRange(0,728);    
    }


    cout << "The starting coordinates are:\n";
    for (int i = 0; i < num; i++)
    {
        cout << "Ship " << i + 1 << ": ";
        cout << "(" << my_ships[i].x << "," << my_ships[i].y << ")" << endl;
    }


    cout << "Simulating movement..." << endl;


    bool checker = inbounds(my_ships,num);


    while(checker == true)
    {
        for (int i = 0; i < num; i++)
        {
            update(my_ships[i]);
        }
    }


    cout << "One of the ships is now out of bounds.\n";
    cout << "The final positions were:\n";
    for (int i = 0; i < num; i++)
    {
        cout << "Ship " << i + 1 << ": ";
        cout << "(" << my_ships[i].x << "," << my_ships[i].y << ")" << endl;
    }    
}


bool inbounds (spaceShip my_ships[], int num)
{
    for (int i = 0; i < num; i++)
    {
        if (my_ships[i].x >= 1024 || my_ships[i].y >= 728)
        {
            return false;
        }
    }


    return true;
}


int randRange (int low, int high) {


    return rand() % (high - low + 1) + low;


}


spaceShip update (spaceShip ship)
{
    int operation = randRange(1,2);
    int movement = randRange(1,10);


    switch (operation)
    {
        case 1: ship.x += movement;
                 ship.y += movement;
                 return ship;
                 break;
         case 2: ship.x -= movement;
                 ship.y -= movement;
                 return ship;
                break;        
    }
}

and this is the output i see:

$ ./ship_struct.exe
How many space ships would you like to simulate?
4
Generating random starting coordinates...
The starting coordinates are:
Ship 1: (308,261)
Ship 2: (387,680)
Ship 3: (500,483)
Ship 4: (577,354)
Simulating movement...


and it just gets stuck there. can anyone tell me why this is happening?

[laserlight]

You should avoid global variables, so you should not declare num at file scope. Rather, it should be declared in the main function.

This declares a variable length array:

spaceShip my_ships[num];

The variable length array feature is not part of standard C++. I suggest that you #include <vector> and use a std::vector<spaceShip> instead.

As for your problem, look carefully at this loop:

while(checker == true)
{
    for (int i = 0; i < num; i++)
    {
        update(my_ships[i]);
    }
}

The loop will keep looping as long as checker is true. But since checker is not changed within the loop, if checker is true, it will always be true, thus you end up with an infinite loop.

[treebeard]

Thanks for the feedback. I don't know about vectors yet really and the book I'm learning from is using this as an example problem for structures and arrays...

so I added this to the code

while(true)
	{
		for (int i = 0; i < num; i++)
		{
			update(my_ships[i]);
		}		
		bool checker = inbounds(my_ships,num);
		if (checker == false)
		{
			break;
		}
	}

but I still get an infinite loop. I'm really stuck on this and I know you aren't supposed to ask for answers but could anyone explain or give me hints as to how I would do this? I wasn't sure how to change it so that checker was changed within the loop

[jiggunjer]

Your update function makes a copy of the ship object, moves it, returns the copy. Since you don't store the return or overwrite the original object the object is trashed and nothing is truly updated. Also make num const.

[laserlight]

Notice that you declared update like this:

spaceShip update(spaceShip ship)

But you did not return a value. Rather, you probably should declare update like this:

void update(spaceShip& ship)

This way, the changes to ship from within the update function will affect the shapeShip object from the caller, and you declare that the function indeed does not return a value.

Furthermore, instead of having the checker variable, you could write:

while (inbounds(my_ships, num))
{
    for (int i = 0; i < num; i++)
    {
        update(my_ships[i]);
    }
}

EDIT:
Oh, I see. There is a return statement that I missed. If you want to return, then keep to the original declaration. However, you should then write it more clearly as:

spaceShip update(spaceShip ship)
{
    int operation = randRange(1,2);
    int movement = randRange(1,10);

    switch (operation)
    {
    case 1:
        ship.x += movement;
        ship.y += movement;
        break;
    case 2:
        ship.x -= movement;
        ship.y -= movement;
        break;
    }
    return ship;
}

Personally, I would go with the reference parameter with void return type since the function is named update.

[treebeard]

Notice that you declared update like this:

spaceShip update(spaceShip ship)

But you did not return a value. Rather, you probably should declare update like this:

void update(spaceShip& ship)

This way, the changes to ship from within the update function will affect the shapeShip object from the caller, and you declare that the function indeed does not return a value.

Thanks for the info. Btw, this may be a stupid question, but did you mean to put the "&" after spaceShip in the declaration? If so, what does the addition of that do?

[Guest]

The & represents a so called reference. If you pass a spaceShip plainly to your function, the function will be working on a copy of it. Since you want to update the original object, you need to pass it as a reference.

void fn1(int n) // takes n "by value"
{
    n = 5;
}

void fn2(int& n) // takes reference to n
{
    n = 5;
}

int main()
{
    int x = 1;
    fn1(x); // creates a copy, works on that
    cout << x << endl; // x is still 1
    fn2(x); // works on reference to original
    cout << x << endl; // x is now 5
}