Drawing a circle segment

Question

I wrote code to draw a circle segment, such that the segment angle will be facing the mouse.

My code:

#include <SFML/Graphics.hpp>
#include <math.h>

const float PI{ 3.14159265359f };

//convert degress to radians
float degToRad(float deg)
{
    return
        deg * PI / 180;
}


void drawCircleSegment(sf::RenderWindow & gameWindow)
{
    gameWindow.clear();
    sf::VertexArray arr(sf::LinesStrip);

    float segmentAngle{degToRad(45)};
    float radius{100};
    sf::Vector2f radiusPos{ gameWindow.getView().getCenter() };

    unsigned circlePoints{20};
    float angle{ 0 };

    arr.append(radiusPos);

    do
    {   
        sf::Vector2f mRelP //get relative mouse position
        {
            sf::Mouse::getPosition(gameWindow).x - radiusPos.x,
            sf::Mouse::getPosition(gameWindow).y - radiusPos.y
        };

        float mouseAngle{ atan2(mRelP.y, mRelP.x) }; //get angle between relative mouse position and circle center

        sf::Vector2f p
        {           
            cos(angle + (mouseAngle - segmentAngle / 2)) * radius, //position = cos(angle + lookAngle) * rad
            sin(angle + (mouseAngle - segmentAngle / 2)) * radius  //position = sin(angle + lookAngle) * rad        
        };

        p += radiusPos;
        arr.append(p);

        angle += segmentAngle / circlePoints;

    } while (angle < segmentAngle);

    arr.append(radiusPos);

    gameWindow.draw(arr);
    gameWindow.display();
}

int main()
{
    sf::RenderWindow gameWindow(sf::VideoMode(800, 600), "circle");
    gameWindow.setFramerateLimit(60);

    while (gameWindow.isOpen())
    {
        sf::Event event;
        while (gameWindow.pollEvent(event))
            if (event.type == sf::Event::Closed)
                gameWindow.close();

        drawCircleSegment(gameWindow);
    }
}

What do you think about it?

Answer 1

Overall, this is pretty good. It's straightforward and to the point. Here are some other thoughts.

Don't Declare PI

You don't need to declare π. It's already declared in math.h as M_PI. Though in C++, it's preferred to #include <cmath> instead, which will put the standard math functions into the std namespace. In C++11 and later, it's suggested to use a calculation to define it such as:

const auto kPi = std::acos(-1.0);

Declare Functions constexpr When Appropriate

You don't say whether you're using C++11 or higher, or not. If you are, I would recommend declaring your degToRad() function constexpr so that it can be used in the definition of constants.

Don't Repeat Yourself

You have some code inside your loop that always calculates the same thing each iteration. The definition of mRelP, and mouseAngle should be moved outside the loop so you do less work in the loop.

Naming

mRelP is a confusing name. In many codebases it's a convention to prepend "m" to member variables. I'd just call it something more readable like mouseRelPos.

Answer 2

Adding to @user1118321's answer:

Less curly brackets in variable definitions

Things like unsigned circlePoints{20}; and float angle{ 0 }; aren't needed. Use unsigned circlePoints = 20; and float angle = 0;

Why?
Because the kind of variable definitions you use are less commonplace and don't make the code any better.

Don't put code in the wrong places.

Avoid using once per frame functions (such as gameWindow.clear(); and gameWindow.display();) in functions other than the main loop. They should go in main instead of drawCircleSegments.

Why?
If you needed to call drawCircleSegments multiple times, the window would clear and display multiple times, which would not work well.