4
\$\begingroup\$

I am writing a game via SFML. In difference places I want to use loaded fonts. So I decided to write the fonts loader, so that load fonts only one time and use them in the future from class. My code:

enum class FontsID
{
    PIXEBOY,
    DIGITAL7
};

class FontObject
{
public:
    FontObject::FontObject(const std::string &texturePath)
    {
        font->loadFromFile(texturePath);
    }
    std::shared_ptr<sf::Font> font = std::make_shared<sf::Font>();
};

class FontsLoader
{
public:
    static FontsLoader &getInstance()
    {
        static FontsLoader instance;
        return instance;
    }
    const std::shared_ptr<sf::Font> getFont(FontsID fontID) const
    {
        return allFonts[static_cast<int>(fontID)].font;
    }

private:
    FontsLoader()
    {
        allFonts.push_back(FontObject("data/fonts/pixeboy.ttf"));
        allFonts.push_back(FontObject("data/fonts/digital-7.ttf"));
    }
    std::vector<FontObject> allFonts;
};
static FontsLoader &fontsLoader = FontsLoader::getInstance();

I don't think using an enum is good solution, for example, enum count may be more or less than allFonts vector items count, adding in the constuctor in proper sequence also not good , but fontsObject.getFont(FontsID::PIXEBOY); is convenient for me, I will never confuse fonts name. Is there a better way?

\$\endgroup\$
2
\$\begingroup\$

FontObject

  • Since all your data members and methods are public, consider making it a struct instead of a class. Makes no difference to a compiler, but makes your intention clearer.

  • Consider using a unique_ptr instead of shared_ptr, since only one instance of GameObject owns the object.

FontsLoader

  • I'm sure they are many people who will explain why you shouldn't use the Singleton pattern. Ultimately, it comes down to whether you (and people you're coding with) are okay with. I've seen various large projects (~100K-1M lines) use the Singleton pattern, so I'm not going to tell you whether you should or shouldn't use it.

  • Use something like an std::unordered_map<FontsID, FontsObject>. That way, you won't have to cast it into an integer and access a vector.

  • On a side note, for vector access, use size_t.

  • Consider passing the sf::Font object as a raw pointer or a const reference. Remember, non-owning raw pointers is FINE, even recommended. Whatever calls the getFont method is not concerned with ownership of the object, just the object itself. Don't pass smart pointers around unless you want the calling code to have ownership of the object.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.