Cart-creation class using SFML

Question

My code is for a simple "cart-creation" class and sample use in SFML. The Cart class inherits sf::RectangleShape and will end up having more member functions (isFull, numberOfWheels, etc.) There will also be a conveyor belt class in the transvac folder. Currently, though, the Cart class is really just a special sort of sf::RectangleShape.

I have had very little experience with design patterns and I would like to know how I can use them in general to make the Cart class more effective. I think the Cart class may be a Decorator, but I'm not sure.

Also, the other important things that I would like to look at are: use of header files, OOP, source tree layout, and general best practices.

Compile with:

g++ -I ./include -Wall -Wextra -Werror -std=c++11 -c ./include/transvac/cart.cpp -o ./obj/cart.o
g++ -I ./include ./src/main.cpp -Wall -Wextra -Werror -std=c++11 -lsfml-graphics -lsfml-window -lsfml-system ./obj/cart.o -o ./bin/cart-mover

Directory tree:

/cart-mover
    /bin
        cart-mover
    /include
        /transvac
             cart.hpp
             cart.cpp
    /obj
        cart.o
    /src
        main.cpp

cart.hpp:

// cart.hpp

#include <SFML/Graphics.hpp>

#ifndef __CART_HPP_INCLUDED__
#define __CART_HPP_INCLUDED__

enum Orientation {VERTICAL, HORIZONTAL};

// TransVac namespace
namespace tvc {

    class Cart : public sf::Drawable {
        // The Cart class is basically a glorified sf::RectangleShape.

        public:
            // Constructor- create with position, color, and orientation.
            Cart(const sf::Vector2f& cart_pos_, sf::Color cart_color_ = sf::Color::Black, Orientation cart_or_ = VERTICAL);

            // The Position and FillColor setters and getters are just wrappers for sf::RectangleShape member methods.
            void setPosition(const sf::Vector2f& cart_pos_);
            const sf::Vector2f& getPosition();

            void setFillColor(sf::Color cart_color_);
            sf::Color getFillColor();

            // Not related to sf::RectangleShape member methods.
            void setOrientation(Orientation cart_or_);
            Orientation getOrientation();

        private:
            // Virtual method needed for proper drawing in a RenderWindow. Overrides inherited Drawable member method.
            virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const;     

            // The heart of the Cart.
            sf::RectangleShape display_rect;

            Orientation cart_or;

            static const int CART_WIDTH = 28.5; // inches
            static const int CART_HEIGHT = 48;  // inches
    };
}

#endif //  __CART_HPP_INCLUDED__

cart.cpp:

// cart.cpp

#include "cart.hpp"

namespace tvc {

    // Everything in here pans out into the setter member methods.
    // Again, much of this is just a wrapper for sf::RectangleShape.
    Cart::Cart(const sf::Vector2f& cart_pos_, sf::Color cart_color_, Orientation cart_or_) {
        if(cart_or_ == VERTICAL)
            display_rect.setSize(sf::Vector2f(CART_WIDTH, CART_HEIGHT));
        else
            display_rect.setSize(sf::Vector2f(CART_HEIGHT, CART_WIDTH));

        display_rect.setPosition(cart_pos_);
        display_rect.setFillColor(cart_color_);

        // These lines are needed to construct a cart every time:

        display_rect.setOutlineThickness(-1); // Outline goes inside cart boundary
        display_rect.setOutlineColor(sf::Color::Black);
    }

    void Cart::setPosition(const sf::Vector2f& cart_pos_) {
            display_rect.setPosition(cart_pos_);
            return;
    }

    const sf::Vector2f& Cart::getPosition() {
        return display_rect.getPosition();
    }

    void Cart::setFillColor(sf::Color cart_color_) {
        display_rect.setFillColor(cart_color_);
        return;
    }

    sf::Color Cart::getFillColor() {
        return display_rect.getFillColor();
    }

    void Cart::setOrientation(Orientation cart_or_) {
        if(cart_or_ == VERTICAL)
            display_rect.setSize(sf::Vector2f(CART_WIDTH, CART_HEIGHT));
        else
            display_rect.setSize(sf::Vector2f(CART_HEIGHT, CART_WIDTH));
        return;
    }

    Orientation Cart::getOrientation() {
        return cart_or;
    }

    // Virtual method- overrides inherited method from sf::Drawable.
    void Cart::draw(sf::RenderTarget& target, sf::RenderStates states) const {
        target.draw(display_rect, states);
        return;
    }
}

main.cpp:

// main.cpp

#include <SFML/System.hpp>
#include <SFML/Graphics.hpp>
#include <SFML/Window.hpp>

#include <random>

#include "transvac/cart.hpp"

const int WIN_HEIGHT = 600; // pixels
const int WIN_WIDTH = 800;  // pixels

// This program creates a randomly colored cart onscreen that can be moved with the arrow keys.
// The program can be closed with "ESC", or the "X" button.

int main() {

    // Set up the window and put it in the upper left corner
    sf::RenderWindow main_window(sf::VideoMode(WIN_WIDTH, WIN_HEIGHT), "Cart Mover", sf::Style::Titlebar | sf::Style::Close);
    main_window.setPosition(sf::Vector2i(0, 0));

    // Make a cart in the middle of the screen and paint it a random color.
    std::default_random_engine generator;
    std::random_device rd;
    generator.seed(rd());
    std::uniform_real_distribution<float> norm_dist(0.0, 255.0);

    tvc::Cart main_cart(sf::Vector2f((WIN_WIDTH - 28.5)/2, (WIN_HEIGHT - 48)/2), \
                        sf::Color(norm_dist(generator), norm_dist(generator), norm_dist(generator)), \
                        VERTICAL);

    // Main processing loop
    while (main_window.isOpen()) {

        sf::Event event;
        // Event processing loop
        while (main_window.pollEvent(event)) {

            switch (event.type) {

                // Close the window if there is an "sf::Event::Closed" event
                case sf::Event::Closed:
                    main_window.close();
                    break;

                // Close the window if the ESC key is pressed
                case sf::Event::KeyPressed:
                    if (event.key.code == sf::Keyboard::Escape)
                        main_window.close();
                        break;

                // No other events are processed
                default:
                    break;
            }

        }

        // Clear background to light grey
        main_window.clear(sf::Color(160, 160, 160));

        main_window.draw(main_cart);

        // If the "right" arrow key is pressed... move the cart right.
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Right)) {
            main_cart.setPosition(sf::Vector2f(main_cart.getPosition().x+1,main_cart.getPosition().y));
        }

        // If the "left" arrow key is pressed... move the cart left.
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Left)) {
            main_cart.setPosition(sf::Vector2f(main_cart.getPosition().x-1,main_cart.getPosition().y));
        }

        // If the "up" arrow key is pressed... move the cart up.
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)) {
            main_cart.setPosition(sf::Vector2f(main_cart.getPosition().x,main_cart.getPosition().y-1));
        }

        // If the "down" arrow key is pressed... move the cart down.
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down)) {
            main_cart.setPosition(sf::Vector2f(main_cart.getPosition().x,main_cart.getPosition().y+1));
        }

        main_window.display();

    }

    return 0;
}

Answer 1

Cart's Design

Cart's interface

Right now, Cart is just a glorified sf::RectangleShape, as you mention. It's probably a good idea to modify its interface so that its methods have cart-ish behavior instead of rectangle-ish behavior. For example, instead of setPosition, you probably want moveForward and moveBackward; instead of getOrientation/setOrientation, you'll want something like getDirection/turnRight/turnLeft. Carts don't change colors for no reason; you may wish to remove setFillColor, or perhaps rename it to repaintCart.

const correctness

When a method is not intended to modify an object (e.g. it inspects the state of the object), you should mark it with const. This has two important benefits:

• When a method is const, the compiler prevents you from accidentally modifying the object's state in the method's implementation.

• When a method is const, you can call the method on a const Cart. This allows you to pass a const Cart& to functions that should be able to inspect a Cart's state but not to change it. It also allows you to call the method from other const methods: For example, with your current implementation you would be unable to call getPosition from within draw.

---

Design Patterns

It's not clear to me how you expect your classes will interact with one another. I can see that Cart has a similar interface right now to sf::RectangleShape; do you intend for your code to treat it like a drawn rectangle? Do you expect the rest of your code to use it with some interface in common with other components you create, or do you expect the rest of your code always to know that it's dealing with a Cart?

You specifically ask about the Decorator pattern. There are two ways that could be relevant to Cart. If you think there's some cart-y property that you could add to any Drawable, or more likely to any Shape or Transformable, then you could implement Cart as some decorator of that interface. For example, you could do this:

// T must be one of ShapeDecorator or sf::Shape. Our job here is complicated by the fact
// that Shape has an extensive nonvirtual interface that would be dangerous to override.
template <typename T>
class ShapeDecorator : public sf::Drawable {
  public:
    static_assert(std::is_base_of<sf::Shape, T>::value
                  || std::is_base_of<ShapeDecorator, T>::value,
                  "T must be one of sf::Shape or ShapeDecorator");

    ShapeDecorator(T* shape) : shape_(shape) { assert(shape_); }

I've chosen to use a raw pointer, with the understanding that the owner of this ShapeDecorator ensures that shape will outlive it. Another possible design would pass and store a std::shared_ptr<T> instead, but it is my opinion that shared_ptr can lead to muddled ownership relationships and tangled designs. See e.g. this programmers.SE question for more discussion).

    ~ShapeDecorator override = default;

    void draw(sf::RenderTarget& target, RenderStates states) const override {
        shape_->draw(target, states);
    }

    // Define the interface that you might want decorators to override as virtual
    virtual void setPosition(const sf::Vector2f& position) {
        shape_->setPosition(position);
    }

    // Define the interface ShapeDecorator will need to provide but that you don't
    // want decorators to override as non-virtual. 
    void setTexture(const Texture* texture, bool resetRect=false) {
        shape_->setTexture(texture, resetRect);
    }
    // etc.

  private:
    T* const shape_;
};

template <typename T>
class Cart : public ShapeDecorator<T> {
  public:
    Cart(T* shape) : ShapeDecorator(shape) {}
    // Whatever overrides you need to make this shape into a Cart.
    void setPosition(const sf::Vector2f& position) override {
        // Carts always seem to be below and to the right of where they actually are.
        ShapeDecorator::setPosition(position + sf::Vector2f(1.0, 2.0));
    }
};

Another way Decorator could be relevant to Cart is if you have some properties that you might add willy-nilly to your various components. For example, you may decide that you want to be able to make your components blink. You'd do this by defining a base Component class, a subclass of Component called Cart, another subclass of Component called ComponentDecorator, and then a specific BlinkComponentDecorator. Since we're defining our own object hierarchy, we can make sure that the only important interfaces that we have are virtual, thus avoiding that nasty template problem we had before:

class Component : public sf::Drawable {
  public:
    ~Component() override = default;

    // Whatever API you think is necessary.
};

class Cart final : public Component {
  public:
    void draw(sf::RenderTarget& target, RenderStates states) const override {
        // Whatever you do to draw a cart
    }
    // ...
};

class ComponentDecorator : public Component {
  public:
    ComponentDecorator(Component* component) : component_(component) { assert(component_); }

    void draw(sf::RenderTarget& target, RenderStates states) const override {
        component_->draw(target, states);
    }
    // Wrap the rest of Component's API

  private:
    Component* component_;
};

class BlinkComponentDecorator final : public ComponentDecorator {
  public:
    BlinkComponentDecorator(Component* component) : ComponentDecorator(component) {}

    void draw(sf::RenderTarget& target, RenderStates states) const override {
        if (std::time(nullptr) % 2) {
            ComponentDecorator::draw(target, states)
        } else {
            // Don't draw anything!
        }
    }
};