Sorting visualizer

Question

I've created simple sorting visualizer in c++ using SFML. I'd love some feedback on how can I improve the code and OOP design.
Whole project: GitHub

main.cpp

#include "controller.hpp"
int main() {
 Controller app;
 while (app.isAppRunning()) {
 app.update();
 }
}

controller.hpp

#pragma once
#include "model.hpp"
#include "sort_context.hpp"
#include "view.hpp"
#include <SFML/Graphics.hpp>
#include <memory>
class Controller {
private:
 SortContext context;
 View view;
 Model model;
 bool keyPressed;
 bool algorithmRunning;
 const unsigned MIN_DELAY = 0;
 const unsigned MAX_DELAY = 100;
 unsigned delay;
 const unsigned MAX_MODEL_SIZE = 1024;
 const unsigned MIN_MODEL_SIZE = 2;
 void increaseModelSize();
 void decreaseModelSize();
 void changeDelay(sf::Event &event);
 void windowClosed();
 void keyPressedEvent(sf::Event &event);
 void keyReleasedEvent();
 void handleEvents();
 void runSortingAlgorithm(std::unique_ptr<SortStrategy> algorithm);
public:
 Controller();
 void update();
 bool isAppRunning() const;
 bool isAlgorithmRunning() const;
};

controller.cpp

#include "controller.hpp"
#include "algorithms/bubble_sort.hpp"
#include "algorithms/insertion_sort.hpp"
#include "algorithms/merge_sort.hpp"
#include "algorithms/selection_sort.hpp"
Controller::Controller()
 : context(SortContext()), view(View(1920, 1080)), model(Model(64)), keyPressed(false), algorithmRunning(false),
 delay(30) {
}
void Controller::windowClosed() {
 this->algorithmRunning = false;
 this->view.closeWindow();
}
void Controller::keyReleasedEvent() {
 this->keyPressed = false;
}
void Controller::changeDelay(sf::Event &event) {
 if (event.mouseWheel.delta > 0)
 this->delay = std::min(this->delay + 10, this->MAX_DELAY);
 if (event.mouseWheel.delta < 0)
 this->delay = std::max(this->delay - 10, this->MIN_DELAY);
}
void Controller::increaseModelSize() {
 unsigned newSize = this->model.size() * 2;
 if (newSize <= this->MAX_MODEL_SIZE)
 this->model.setSize(newSize);
}
void Controller::decreaseModelSize() {
 unsigned newSize = this->model.size() / 2;
 if (newSize >= this->MIN_MODEL_SIZE)
 this->model.setSize(newSize);
}
void Controller::runSortingAlgorithm(std::unique_ptr<SortStrategy> algorithm) {
 this->algorithmRunning = true;
 this->context.setStrategy(std::move(algorithm));
 this->context.sort(this->model, *this);
 this->algorithmRunning = false;
}
void Controller::update() {
 this->handleEvents();
 this->view.updateScreen(this->model);
 this->view.delay(this->delay);
}
bool Controller::isAppRunning() const {
 return this->view.isWindowOpened();
}
bool Controller::isAlgorithmRunning() const {
 return this->algorithmRunning;
}
void Controller::handleEvents() {
 sf::Event event;
 while (this->view.getEvent(event)) {
 switch (event.type) {
 case sf::Event::Closed:
 this->windowClosed();
 break;
 case sf::Event::KeyPressed:
 this->keyPressedEvent(event);
 break;
 case sf::Event::KeyReleased:
 this->keyReleasedEvent();
 break;
 case sf::Event::MouseWheelMoved:
 this->changeDelay(event);
 break;
 default:
 break;
 }
 }
}
void Controller::keyPressedEvent(sf::Event &event) {
 if (this->keyPressed)
 return;
 this->keyPressed = true;
 if (event.key.code == sf::Keyboard::T)
 this->algorithmRunning = false;
 if (this->algorithmRunning)
 return;
 switch (event.key.code) {
 case sf::Keyboard::S:
 this->model.shuffle();
 break;
 case sf::Keyboard::Hyphen:
 this->decreaseModelSize();
 break;
 case sf::Keyboard::Equal:
 this->increaseModelSize();
 break;
 case sf::Keyboard::Num1:
 this->runSortingAlgorithm(std::make_unique<BubbleSort>());
 break;
 case sf::Keyboard::Num2:
 this->runSortingAlgorithm(std::make_unique<SelectionSort>());
 break;
 case sf::Keyboard::Num3:
 this->runSortingAlgorithm(std::make_unique<InsertionSort>());
 break;
 case sf::Keyboard::Num4:
 this->runSortingAlgorithm(std::make_unique<MergeSort>());
 break;
 default:
 break;
 }
}

model.hpp

#pragma once
#include <algorithm>
#include <random>
#include <vector>
class Model {
private:
 struct Sortable {
 private:
 unsigned value;
 bool highlighted;
 std::reference_wrapper<Model> container;
 public:
 Sortable() = delete;
 Sortable(unsigned value, Model &model) : value(value), highlighted(false), container(model){};
 void highlight() {
 this->highlighted = true;
 }
 void unhighlight() {
 this->highlighted = false;
 }
 bool isHighlighted() const {
 return this->highlighted;
 }
 float heightAsPercentage() const {
 return static_cast<float>(this->value) / this->container.get().size();
 }
 void moveToIndex(size_t index) {
 this->container.get().moveToIndex(*this, index);
 }
 bool operator<(Sortable const &rhs) const {
 return this->value < rhs.value;
 }
 bool operator>(Sortable const &rhs) const {
 return this->value > rhs.value;
 }
 bool operator>=(Sortable const &rhs) const {
 return this->value >= rhs.value;
 }
 bool operator<=(Sortable const &rhs) const {
 return this->value <= rhs.value;
 }
 };
 static std::default_random_engine rng;
 std::vector<Sortable> data;
 void moveToIndex(Sortable const &value, size_t newIndex);
public:
 Model(unsigned initSize = 64);
 size_t size() const;
 void setSize(unsigned size);
 void shuffle();
 Sortable operator[](size_t i) const;
 Sortable &operator[](size_t i);
};

model.cpp

#include "model.hpp"
std::default_random_engine Model::rng{std::random_device()()};
Model::Model(unsigned initSize) {
 this->setSize(initSize);
}
size_t Model::size() const {
 return this->data.size();
}
void Model::setSize(unsigned size) {
 this->data.clear();
 for (unsigned i = 1; i <= size; i++) {
 this->data.emplace_back(Sortable(i, *this));
 }
}
void Model::shuffle() {
 std::shuffle(this->data.begin(), this->data.end(), Model::rng);
}
Model::Sortable &Model::operator[](size_t i) {
 return this->data[i];
}
Model::Sortable Model::operator[](size_t i) const {
 return this->data[i];
}
void Model::moveToIndex(Sortable const &value, size_t newIndex) {
 auto it = std::find_if(this->data.begin(), this->data.end(), [&](Sortable const &el) { return &el == &value; });
 if (it == this->data.end() || newIndex >= this->size())
 return;
 size_t oldIndex = std::distance(this->data.begin(), it);
 if (oldIndex < newIndex) {
 std::rotate(this->data.begin() + oldIndex, this->data.begin() + oldIndex + 1, this->data.begin() + newIndex + 1);
 } else {
 std::rotate(this->data.begin() + newIndex, this->data.begin() + oldIndex, this->data.begin() + oldIndex + 1);
 }
}

view.hpp

#pragma once
#include "model.hpp"
#include <SFML/Graphics.hpp>
class View {
private:
 const unsigned WIDTH, HEIGHT;
 const unsigned FRAME_RATE = 60;
 sf::RenderWindow window;
 void draw(const Model &model);
public:
 View(const unsigned WIDTH, const unsigned HEIGHT);
 void delay(unsigned time) const;
 void updateScreen(const Model &model);
 bool getEvent(sf::Event &event);
 bool isWindowOpened() const;
 void closeWindow();
};

view.cpp

#include "view.hpp"
View::View(const unsigned WIDTH, const unsigned HEIGHT)
 : WIDTH(WIDTH), HEIGHT(HEIGHT), window(sf::VideoMode(WIDTH, HEIGHT), "Sorting visualization") {
 window.setFramerateLimit(this->FRAME_RATE);
}
void View::delay(unsigned time) const {
 sf::sleep(sf::milliseconds(time));
}
void View::draw(const Model &model) {
 float width = static_cast<float>(this->WIDTH) / model.size();
 for (size_t i = 0; i < model.size(); i++) {
 float height = this->HEIGHT * model[i].heightAsPercentage();
 sf::RectangleShape rect({width, height});
 rect.setPosition({i * width, this->HEIGHT - height});
 if (model[i].isHighlighted())
 rect.setFillColor(sf::Color::Red);
 else
 rect.setFillColor(sf::Color::Black);
 this->window.draw(rect);
 }
}
void View::updateScreen(const Model &model) {
 this->window.clear(sf::Color::White);
 this->draw(model);
 this->window.display();
}
bool View::getEvent(sf::Event &event) {
 return this->window.pollEvent(event);
}
bool View::isWindowOpened() const {
 return this->window.isOpen();
}
void View::closeWindow() {
 this->window.close();
}

sort_context.hpp

#pragma once
#include "algorithms/sort_strategy.hpp"
#include "model.hpp"
#include <memory>
class Controller;
class SortContext {
private:
 std::unique_ptr<SortStrategy> strategy;
public:
 void setStrategy(std::unique_ptr<SortStrategy> newStrategy);
 void sort(Model &model, Controller &controller);
};

sort_context.cpp

#include "sort_context.hpp"
void SortContext::sort(Model &model, Controller &controller) {
 if (this->strategy)
 this->strategy->sort(model, controller);
}
void SortContext::setStrategy(std::unique_ptr<SortStrategy> newStrategy) {
 this->strategy = std::move(newStrategy);
}

sort_strategy.cpp

#pragma once
#include "../model.hpp"
#define HIGHLIGHT_COMPARED_AND_UPDATE(first, second, controller) \
 if (!controller.isAlgorithmRunning()) \
 return; \
 first.highlight(); \
 second.highlight(); \
 controller.update(); \
 first.unhighlight(); \
 second.unhighlight();
class Controller;
class SortStrategy {
public:
 SortStrategy() = default;
 virtual void sort(Model &model, Controller &controller) const = 0;
 virtual ~SortStrategy() = default;
};

bubble_sort.cpp

#include "algorithms/bubble_sort.hpp"
#include "controller.hpp"
void BubbleSort::sort(Model &model, Controller &controller) const {
 unsigned changes = 1;
 while (changes > 0) {
 changes = 0;
 for (size_t i = 0; i < model.size() - 1; i++) {
 if (model[i + 1] < model[i]) {
 changes++;
 std::swap(model[i], model[i + 1]);
 }
 HIGHLIGHT_COMPARED_AND_UPDATE(model[i], model[i + 1], controller);
 }
 }
}

• pacmaninbw: In the future it would be better to add a follow up question with a link back to the original question rather than editing the question. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered.

Answer 1

 const unsigned MIN_DELAY = 0;
 const unsigned MAX_DELAY = 100;

You don't need to make instance variables since these are the same in any/every instance. So also make them static. But, today use constexpr instead. The adage is constexpr is the new static const.

---

void Controller::windowClosed() {
 this->algorithmRunning = false;
 this->view.closeWindow();
}

🌟 don't use this-> everywhere.🌟 Members are in scope. This is anti-idiomatic in C++.

---

Prefer signed values. See ES.102 and ES.106

---

void Controller::changeDelay(sf::Event &event) {
 if (event.mouseWheel.delta > 0)
 this->delay = std::min(this->delay + 10, this->MAX_DELAY);
 if (event.mouseWheel.delta < 0)
 this->delay = std::max(this->delay - 10, this->MIN_DELAY);
}

It's idiomatic C++ to write the & with the type not with the variable.
In this function, you are not modifying event so why isn't it const?

---

 bool operator<(Sortable const &rhs) const {
 return this->value < rhs.value;
 }
 bool operator>(Sortable const &rhs) const {
 return this->value > rhs.value;
 }
 bool operator>=(Sortable const &rhs) const {
 return this->value >= rhs.value;
 }
 bool operator<=(Sortable const &rhs) const {
 return this->value <= rhs.value;
 }

Are you using a current compiler (C++20)? Just write operator<=>. For older compilers, since the only thing you are doing is calling sort you only need operator< (unless your visualizer is calling various others? Like sort, the working of map, etc., it could be written to only ever use <).

Use noexcept as well for (possibly) better performance.

---

#define HIGHLIGHT_COMPARED_AND_UPDATE(first, second, controller) \
 if (!controller.isAlgorithmRunning()) \
 return; \
 first.highlight(); \
 second.highlight(); \
 controller.update(); \
 first.unhighlight(); \
 second.unhighlight();

Ummm.... yuck?
This is in a CPP file and I don't see it mentioned in the few remaining lines of the file, so what's it doing here?

• Gh0st: Thank you for feedback! I created this macro because I was using this part of code in every algorithm implementation and I didn't want to repeat myself. I added an example of such implementation to the post.
• Gh0st: Sorry, I made a mistake, sort_strategy is an hpp file. Every sorting algorithm inherits from this class.
• JDługosz: @Gh0st Why is it a macro, rather than a function? I don't see anything about it that wouldn't work as a function.
• Gh0st: The only problem is this if statement. Should I just write these two lines in every algorithm or there is a better way to stop a function?
• JDługosz: Two lines in each function is clear. Or, if there's nothing else in the function that would contain this, just put the highlight etc. inside an if block instead so it's skipped if not running.