Conflict between SRP and OCP

[root-kidik]

Prerequisite Checklist

• I searched for existing issues to prevent duplicates
• I searched for existing discussions on the forum to prevent duplicates
• I understand SFML's scope and believe the feature fits it

Describe your feature request here

sf::Sprite, sf::Text, sf::VertexArray ...: break SRP rules, for follow OCP
Have two reason for change.
draw() - for follow OCP
other

Use Cases

make friends SRP with OCP

API Example

//-----------------------------------------------------------------------------
//
// Source code for MIPT ILab
// Slides: https://sourceforge.net/projects/cpp-lects-rus/files/cpp-graduate/
// Licensed after GNU GPL v3
//
//-----------------------------------------------------------------------------
//
//  Demonstration of parent inversion: complete SRP and OCP satisfied
//
//-----------------------------------------------------------------------------

#include <algorithm>
#include <iostream>
#include <memory>
#include <string>
#include <utility>
#include <vector>

using Screen = std::ostream;

struct Vector3D {
  int x, y, z;
};

struct Polygon3D {
  std::vector<Vector3D> vs;
  Polygon3D(std::initializer_list<Vector3D> il) : vs(il) {}
};

void draw(int x, Screen &out, size_t position) {
  out << std::string(position, ' ') << x << "\n";
}

void draw(Vector3D v, Screen &out, size_t position) {
  out << std::string(position, ' ') << "(" << v.x << ", " << v.y << ", " << v.z
      << ")\n";
}

void draw(const Polygon3D &p, Screen &out, size_t position) {
  out << std::string(position, ' ') << "<polygon>" << std::endl;
  for (auto v : p.vs)
    ::draw(v, out, position + 2);
  out << std::string(position, ' ') << "</polygon>" << std::endl;
}

class Drawable {
  struct IDrawable {
    virtual ~IDrawable() = default;
    virtual std::unique_ptr<IDrawable> copy_() const = 0;
    virtual void draw_(Screen &, size_t) const = 0;
  };

  template <typename T> struct DrawableObject final : IDrawable {
    T data_;
    DrawableObject(T x) : data_(std::move(x)) {}
    std::unique_ptr<IDrawable> copy_() const override {
      return std::make_unique<DrawableObject>(*this);
    }
    void draw_(Screen &out, size_t position) const override;
  };

  std::unique_ptr<IDrawable> self_;

public:
  template <typename T>
  Drawable(T x) : self_(std::make_unique<DrawableObject<T>>(std::move(x))) {}

  // copy ctor, move ctor and assignment
public:
  Drawable(const Drawable &x) : self_(x.self_->copy_()) {}
  Drawable(Drawable &&x) noexcept = default;
  Drawable &operator=(Drawable x) noexcept {
    self_ = std::move(x.self_);
    return *this;
  }

public:
  friend void draw(const Drawable &x, Screen &out, size_t position) {
    x.self_->draw_(out, position);
  }
};

using Model = std::vector<Drawable>;

void draw(const Model &x, Screen &out, size_t position) {
  out << std::string(position, ' ') << "<world>" << std::endl;
  for (const auto &e : x)
    draw(e, out, position + 2);
  out << std::string(position, ' ') << "</world>" << std::endl;
}

template <typename T>
void Drawable::DrawableObject<T>::draw_(Screen &out, size_t position) const {
  ::draw(data_, out, position);
}

int main() {
  Model document;
  document.push_back(0);
  document.push_back(Polygon3D{{2, 1, 6}, {-3, 7, 4}});
  document.push_back(document);
  document.push_back(Vector3D{-3, 7, 4});
  draw(document, std::cout, 0);
}

[ChrisThrasher]

You appear to be referring to SOLID principles. We don't oppose those rules but we also don't dogmatically follow them. This doesn't look like something we're interested in doing.

[eXpl0it3r]

I don't particularly see a open-closed principle violation for draw() as it's extendible via sf::Drawable and inheritance if necessary. You should really be more specific, i.e. spend more than 5min writing down your thought process and reasoning (DeepL, ChatGPT and Google Translate all understand Russian as well).

One of SFML's "core goal" is to provide a simple API. This means we're not in this to practice perfect SOLID principles, but we're gladly trading something like single responsibility principle for a simpler API. Yes, we could extract some "SpriteDrawer" class or what else, to move the responsibility out of the sprite class, but that would make the API a lot more complicated, for no added benefit at this point. We might end up having to change the API for something like batch drawing, but this will bring its own challenges disconnected from SOLID principles.

The code you showed is nice a theoretical example, where you don't have to care about states and references, but it already shows an API that's a lot harder to understand and even breaks encapsulation, as implementation details such as draw_() and copy_() are exposed on a public API/interface.