1
\$\begingroup\$

using XNA and C# I built a game as a tutorial, but I have since added functionality to allow the player to move around on a world map which requires the camera to "move" but I no longer like the look of the if statements.

is there some way that I can clean this up?

private static Rectangle scrollArea = new Rectangle(150, 150, 500, 300);

private void repositionCamera(GameTime gameTime, Vector2 moveAngle)
{
    float elapsed = (float)gameTime.ElapsedGameTime.TotalSeconds;
    float moveScale = playerSpeed * elapsed;

    if ((playerSprite.ScreenRectangle.X < scrollArea.X) && (moveAngle.X < 0))
    {
        Camera.Move(new Vector2(moveAngle.X, 0) * moveScale);
    }

    if ((playerSprite.ScreenRectangle.Right > scrollArea.Right) && (moveAngle.X > 0))
    {
        Camera.Move(new Vector2(moveAngle.X, 0) * moveScale);
    }

    if ((playerSprite.ScreenRectangle.Y < scrollArea.Y) && (moveAngle.Y < 0))
    {
        Camera.Move(new Vector2(0, moveAngle.Y) * moveScale);
    }

    if ((playerSprite.ScreenRectangle.Bottom > scrollArea.Bottom) && (moveAngle.Y > 0))
    {
        Camera.Move(new Vector2(0, moveAngle.Y) * moveScale);
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

You can generalize bodies of ifs:

private static void MoveCamera(float x, float y, float moveScale)
{
    Camera.Move(new Vector2(x, y) * moveScale);
}

private void repositionCamera(GameTime gameTime, Vector2 moveAngle)
{
    float elapsed = (float)gameTime.ElapsedGameTime.TotalSeconds;
    float moveScale = playerSpeed * elapsed;

    if ((playerSprite.ScreenRectangle.X < scrollArea.X) && (moveAngle.X < 0))
    {
        MoveCamera(moveAngle.X, 0, moveScale);
    }

    if ((playerSprite.ScreenRectangle.Right > scrollArea.Right) && (moveAngle.X > 0))
    {
        MoveCamera(moveAngle.X, 0, moveScale);
    }

    if ((playerSprite.ScreenRectangle.Y < scrollArea.Y) && (moveAngle.Y < 0))
    {
        MoveCamera(0, moveAngle.Y, moveScale);
    }

    if ((playerSprite.ScreenRectangle.Bottom > scrollArea.Bottom) && (moveAngle.Y > 0))
    {
        MoveCamera(0, moveAngle.Y, moveScale);
    }
}

Also I suggest to extract all those conditions to properties with clear names like:

private static bool IsCameraOutOfLeft =>
    playerSprite.ScreenRectangle.X < scrollArea.X && moveAngle.X < 0;

private static bool IsCameraOutOfRight =>
    playerSprite.ScreenRectangle.Right > scrollArea.Right && moveAngle.X > 0;

...

Then finally the repositionCamera method will be

private void repositionCamera(GameTime gameTime, Vector2 moveAngle)
{
    float elapsed = (float)gameTime.ElapsedGameTime.TotalSeconds;
    float moveScale = playerSpeed * elapsed;

    if (IsCameraOutOfLeft || IsCameraOutOfRight)
    {
        MoveCamera(moveAngle.X, 0, moveScale);
    }

    if (IsCameraOutOfTop || IsCameraOutOfBottom)
    {
        MoveCamera(0, moveAngle.Y, moveScale);
    }
}

Also a method's name should be PascalCasedRepositionCamera.

\$\endgroup\$
5
  • \$\begingroup\$ ah, yes, back before I knew the good naming standards...lol \$\endgroup\$
    – Malachi
    Aug 25, 2017 at 16:10
  • \$\begingroup\$ @Malachi if you don't know them anymore then you shoud return some of your rep :-P \$\endgroup\$
    – t3chb0t
    Aug 25, 2017 at 17:14
  • \$\begingroup\$ I know them now, but when I first wrote this code I didn't know them well \$\endgroup\$
    – Malachi
    Aug 25, 2017 at 17:49
  • \$\begingroup\$ lol, Visual Studio 2017 actually fixed the naming violation for me with a click of the mouse. \$\endgroup\$
    – Malachi
    Aug 27, 2017 at 0:19
  • \$\begingroup\$ I am still working on this method, you have given me a good starting point to cleaning it up though, thank you \$\endgroup\$
    – Malachi
    Aug 28, 2017 at 13:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.