[SOLVED] Possible memory leak?

[short]

Hi everyone, one question. The function I posted below is called every frame, and throughout it a constructor for a Rectangle class that I made is called, allocating memory. I was wondering, at the end I delete paddleRect and ballRect, but I was not sure if I was creating a huge memory leak.

Is my way of doing this wrong (deleting the rectangles at the end)? I was thinking about writing some methods that are used to set the different corners instead of calling the constructor over and over. Any idea?

Code: Select all

bool collisionWithPaddleArray(GameObject * object, GameObject * ball)
{
	 Rectangle * paddleRect = NULL;
	 Rectangle * ballRect = NULL;
	 for(int i = 0; i < NUM_PADDLES; i++)
	 {
		  paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(), 
			   (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
		  ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

		  if(doRectanglesCollide(paddleRect, ballRect))
		  {
			   BALL_SPEED_X *= -1;
			   BALL_SPEED_Y *= -1;

			   bounceBall(ball);

			   if(ball->get_X_Direction() == RIGHT)
			   {
					while(doRectanglesCollide(paddleRect, ballRect) == true)
					{
						 paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(), 
							  (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
						 ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

						 moveGameObjectOnePixel(ball,MOVE_LEFT, DO_NOT_MOVE);
					}
			   }
			   else if(ball->get_X_Direction() == LEFT)
			   {
					while(doRectanglesCollide(paddleRect, ballRect) == true)
					{
						 paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(), 
							  (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
						 ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

						 moveGameObjectOnePixel(ball, MOVE_RIGHT, DO_NOT_MOVE);
					}
			   }
			   else
			   {
					myLog->writeToFile("ball->get_X_Direction() is not equal to east or west. \n");
			   }

			   if(ball->get_Y_Direction() == UP)
			   {
					while(doRectanglesCollide(paddleRect, ballRect) == true)
					{
						 paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(), 
							  (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
						 ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

						 moveGameObjectOnePixel(ball, DO_NOT_MOVE, MOVE_DOWN);
					}
			   }
			   else if(ball->get_Y_Direction() == DOWN)
			   {
					while(doRectanglesCollide(paddleRect, ballRect) == true)
					{
						 paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(), 
							  (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
						 ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

						 moveGameObjectOnePixel(ball, DO_NOT_MOVE, MOVE_UP);
					}
			   }
			   else
			   {
					myLog->writeToFile("ball->get_Y_Direction() is not equal to north or south. \n");
			   }

			   delete(paddleRect); 
			   delete(ballRect); // cleanup
			   return true;
			   

		  }
	 }
	 delete(paddleRect);
	 delete(ballRect); // cleanup
	 return false;

}

[MarauderIIC]

Yes, you're generating massive amounts of memory leaks. For every time your program executes a new, your program must execute an accompanying delete. Among other places, you have

Code: Select all

while(doRectanglesCollide(paddleRect, ballRect) == true)
               {
                   paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(),
                       (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
                   ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

                   moveGameObjectOnePixel(ball,MOVE_LEFT, DO_NOT_MOVE);
               }

Which executes a bunch of news but no deletes for them. This should be something more like

Code: Select all

while(doRectanglesCollide(paddleRect, ballRect) == true)
               {
                   delete paddleRect; //delete the rectangles that already exist
                   delete ballRect;
                   paddleRect = new Rectangle(paddleArray[i]->getX(), paddleArray[i]->getY(),
                       (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight()));
                   ballRect = new Rectangle(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

                   moveGameObjectOnePixel(ball,MOVE_LEFT, DO_NOT_MOVE);
               }
               delete paddleRect; //delete the rectangles that were allocated in the last iteration of the loop
               delete ballRect;
               paddleRect = ballRect = NULL;

Note that if you delete a NULL object, there is no adverse effect. If you delete an object that has been deleted, your program will generate an exception. Although I don't see why you can't just do the much safer

Code: Select all

for (...) {
Rectangle paddleRect(all necessary stuff);
Rectangle ballRect(all necessary stuff);
...
    while(doRectanglesCollide(&paddleRect, &ballRect) == true)
               {
                   paddleRect.setAll(paddleArray[i]->getX(), paddleArray[i]->getY(),
                       (paddleArray[i]->getX() + paddleArray[i]->getWidth()), (paddleArray[i]->getY() + paddleArray[i]->getHeight());
                   ballRect.setAll(ball->getX(), ball->getY(), ( ball->getX() + ball->getWidth() ), ( ball->getY() + ball->getHeight() ) );

                   moveGameObjectOnePixel(ball,MOVE_LEFT, DO_NOT_MOVE);
               }

[short]

Ahhhhh, thank you MarauderIIRC.

[MarauderIIC]

Anytime.