Ping Pong game in SFML

Question

I am learning C++ and SFML so I want to know if I am laying my code out neatly and if my code is efficient. I know this is not a full game yet but I want to know if I am on the right tracks.

#include <SFML/Graphics.hpp>
#include "ResourcePath.hpp"

#include <iostream>

class Paddel {
    int speed = 3;
    sf::Vector2f border;
    sf::Vector2f size;
    sf::Vector2f position;
    sf::Vector2f screenSize;
public:
    sf::RectangleShape shape;

    Paddel(sf::Vector2f screenSize,int player)
    {
        this->screenSize = screenSize;
        this->size = sf::Vector2f(20,100);
        this->border = sf::Vector2f(8,6);

        if(player == 1)
            this->position = border;
        else if(player == 2) {
            position.x = (screenSize.x - size.x) - border.x;
            position.y = border.y;
        }

        this->shape.setSize(this->size);
        this->shape.setPosition(this->position);
    }

    void moveUp()
    {
        this->position.y -= this->speed;
        if(this->position.y < this->border.y)
            this->position.y += this->speed;
        this->shape.setPosition(this->position);
    }

    void moveDown()
    {
        this->position.y += this->speed;
        //100 - (600 + 5)
        if(this->position.y > (this->screenSize.y - this->border.y) - this->size.y)
            this->position.y -= this->speed;
        this->shape.setPosition(this->position);
    }
};

using std::cout;
using std::endl;


int main()
{
    sf::Vector2f screenSize(800,600);
    sf::RenderWindow Window(sf::VideoMode(screenSize.x,screenSize.y),"Ping Pong");
    Window.setFramerateLimit(60);
    Window.setVerticalSyncEnabled(true);

    Paddel playerOne(screenSize,1);
    Paddel playerTwo(screenSize,2);

    const sf::Time TimePerFrame = sf::seconds(1.f/60.f);
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;
    sf::Time elapsedTime;

    bool play = true;

    while(Window.isOpen())
    {
        Window.clear();
        sf::Event Event;

        while(Window.pollEvent(Event))
        {
            if(Event.type == sf::Event::Closed)
                Window.close();
        }

        elapsedTime = clock.restart();
        timeSinceLastUpdate += elapsedTime;
        if(play){
            if(timeSinceLastUpdate > TimePerFrame){
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::W))
                {
                    playerOne.moveUp();
                }
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::S))
                {
                    playerOne.moveDown();
                }

                if(sf::Keyboard::isKeyPressed(sf::Keyboard::Up))
                {
                    playerTwo.moveUp();
                }
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::Down))
                {
                    playerTwo.moveDown();
                }

                if(sf::Keyboard::isKeyPressed(sf::Keyboard::P))
                    play = false;
            }

            Window.draw(playerOne.shape);
            Window.draw(playerTwo.shape);

            Window.display();
        }
    }
}

Answer 1

your code is fine but I would go with some thing like this, i have commented the changes

#include <SFML/Graphics.hpp>
#include <cstdint>

// for eliminating magic numbers
enum class Players
{
    PlayerOne,
    PlayerTow
};

enum class ScreenSize
{
    Width = 800,
    Height = 600
};

// SFML has tiny class for rendering purpose, it has only a one virtual fucntion 
class Paddel : public sf::Drawable
{
    // for member data perfered tobe started with m prefix 
    float mSpeed; // here is an optional you may use integer value
    sf::Vector2f mBorder;
    sf::Vector2f mPosition;
    sf::Vector2u mScreenSize;
    sf::RectangleShape mShape; // <-- now it's private
public:

    Paddel(sf::Vector2u screenSize, Players player)
        // member data perfered be initilaized as contructor's initilaized list
        : mScreenSize(screenSize)
        , mBorder(8, 6)
        , mSpeed(3.f)
    {
        sf::Vector2f size = sf::Vector2f(20, 100);


        switch (player)
        {
        case Players::PlayerOne:
            mPosition = mBorder;
            break;
        default:
            mPosition.x = (mScreenSize.x - size.x) - mBorder.x;
            mPosition.y = mBorder.y;
        }

        mShape.setSize(size);
        mShape.setPosition(mPosition);
    }

    void moveUp()
    {
        mPosition.y -= mSpeed;
        if (mPosition.y < mBorder.y)
            mPosition.y += mSpeed;
        mShape.setPosition(mPosition);
    }

    void moveDown()
    {
        mPosition.y += mSpeed;
        //100 - (600 + 5)
        if (mPosition.y >(mScreenSize.y - mBorder.y) - mShape.getSize().y)
            mPosition.y -= mSpeed;
        mShape.setPosition(mPosition);
    }

    // sf::Drawable has singe virtual fucntion for rendering, override it 
    void draw(sf::RenderTarget& target, sf::RenderStates states) const override
    {
        // sf::RectangleShape has its own defualt RenderStates
        target.draw(mShape, states);
    }
};

int main()
{
    // unfortunately strong type enumeration doesn't decay to integer value, so that we need to static cast it to integer type 
    // here we don't want to have a negative value for size, we should use unsigned int or use std::uint32_t which is declared 
    // in <cstdint> header file if you considered C++11 features.
    sf::Vector2u screenSize(static_cast<std::uint32_t>(ScreenSize::Width), static_cast<std::uint32_t>(ScreenSize::Height));
    sf::RenderWindow Window(sf::VideoMode(screenSize.x, screenSize.y), "Ping Pong");

    Window.setFramerateLimit(60);
    Window.setVerticalSyncEnabled(true);

    Paddel playerOne(screenSize, Players::PlayerOne); 
    Paddel playerTwo(screenSize, Players::PlayerTow);

    const sf::Time TimePerFrame = sf::seconds(1.f / 60.f);
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;
    sf::Time elapsedTime;

    bool play = true;

    while (Window.isOpen())
    {
        Window.clear();
        sf::Event Event;

        while (Window.pollEvent(Event))
        {
            if (Event.type == sf::Event::Closed)
                Window.close();
        }

        elapsedTime = clock.restart();
        timeSinceLastUpdate += elapsedTime;

        if (play)
        {
            if (timeSinceLastUpdate > TimePerFrame)
            {
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::W))
                {
                    playerOne.moveUp();
                }
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::S))
                {
                    playerOne.moveDown();
                }

                if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up))
                {
                    playerTwo.moveUp();
                }
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down))
                {
                    playerTwo.moveDown();
                }

                if (sf::Keyboard::isKeyPressed(sf::Keyboard::P))
                    play = false;
            }

            Window.draw(playerOne); // <-- we can draw Paddel object directly
            Window.draw(playerTwo);

            Window.display();
        }
    }
}