# SMFL Dice class

I'm writing game in SFML and I got question about class design. For example, I have a dice class that looks like this:

#include <SFML/Graphics.hpp>
#include <SFML/Audio.hpp>
#include "DEFINITIONS.h"
#include <random>
#include <chrono>

class Dice {
std::default_random_engine rng;
sf::Texture m_texture;
sf::RectangleShape m_dice;
sf::SoundBuffer m_buffer;
sf::Sound m_sound;
public:
Dice(float positionX, float positionY);
~Dice() = default;
int roll();
inline sf::RectangleShape& getShape() { return m_dice; }
inline void setTexture(std::string filePath) { m_texture.loadFromFile(filePath); }
};

Dice::Dice(float positionX, float positionY) {
m_dice.setSize(sf::Vector2f(DICE_WIDTH, DICE_HEIGHT));
m_dice.setPosition(positionX, positionY);
m_dice.setTexture(&m_texture);
m_sound.setBuffer(m_buffer);
}

int Dice::roll() {
m_sound.play();

rng.seed(std::chrono::system_clock::now().time_since_epoch().count());
std::uniform_int_distribution<int> dist(1, 6);
int number = dist(rng);

std::string filePath = "resources//dice" + std::to_string(number) + ".png";
m_dice.setTexture(&m_texture);

return number;
}


And few other classes are designed in a similar way. For example, Button has sf::Texture which depends on the bool enabled; variable. A colleague of mine, who has a bit more experience with programming, told me that he isn't sure about that solution. In his opinion it's better to split classes into pure logic and pure view, for example DiceLogic and DiceView, or at least put logic and view in separate methods and then call them in another one. What's the best solution here to maintain clean code?

## Think carefully about class design

The main question you've asked here is about class design. It often does make sense to separate the logic from the presentation of the data. Reasons for doing so include:

1. simplifies any later adaptation (e.g. porting to some non-SFML platform)
2. makes automated testing simpler
3. may be simpler to understand

On the other hand:

1. YAGNI
2. this is a really small class
3. may complicate understanding with proliferation of classes

I could argue it either way for this one, but I would offer this suggestion: if you leave it as a single class, at least separate the operation from the presentation. That is, roll could return the appropriate int but I'd suggest deriving Dice from sf::Drawable and implementing a separate draw() function. Generally, I'd lean in the direction of having separate classes for the logic and presentation, even with such a small class. I find that it often helps with maintenance.

## Seed the random number generator exactly once

The existing class reseeds the random number generator each time roll is called. Not only does that reduce the randomness, but the seed is not particularly random, either. Better might be to use std::random_device if your compiler (and hardware) supports it.

The class is called Dice but "dice" is the plural of "die" and this class actually appears to implement a single die. I'm not going to strongly recommend changing it to Die (because that's also a verb which might confuse someone) but it's worth thinking about.
## Understand the use of inline
The inline keyword is often misconstrued as something that will enhance the performance of the compiled code. Typically, it won't. If you're using it because you want to define the functions in the header, I'd suggest that you are probably better off splitting the implementation and the interface into .cpp and .h files instead. See this answer for details.