C++ wrapper for font opaque pointer

Question

I'm creating a class which has a raw pointer member which comes from a C function. In order to make it RAII approved, I just created a method free() that is called on the destructor of this class.

Reading some guidelines I came across that Rule of 5 (and Rule of 3) which, from my understanding, I should also create a bunch of constructors and operator overloadings.

This is what I've done:

/* header */
class SDL2_Font {
 public:
  SDL2_Font() {}
  SDL2_Font(const SDL2_Font& other);  // copy constructor
  SDL2_Font(SDL2_Font&& other);       // move constructor
  ~SDL2_Font() { free(); }

  SDL2_Font& operator=(const SDL2_Font& other); // copy assignment
  SDL2_Font& operator=(SDL2_Font&& other);      // move assignment
 
  bool loadFont(const std::string& path, int size);
  // more stuff
 private:
  void free();

  TTF_Font* font_ = nullptr;
  FontInfo font_info_ {};
  std::string path_ {};
  int size_ = 0;
}


/* cpp file */
SDL2_Font::SDL2_Font(const SDL2_Font& other) {
  if (other.font_ != nullptr) {
    SDL2_Font::loadFont(other.path_, other.size_);
  }
}

SDL2_Font::SDL2_Font(SDL2_Font&& other): font_{other.font_} {
  other.font_ = nullptr;
}

SDL2_Font& SDL2_Font::operator=(const SDL2_Font& other) {
  if (&other != this && other.font_ != nullptr) {
    SDL2_Font::loadFont(other.path_, other.size_);
  }
  return *this;
}

SDL2_Font& SDL2_Font::operator=(SDL2_Font&& other) {
  if (&other != this) {
    SDL2_Font::free();
    font_ = other.font_;
    other.font_ = nullptr;
  }
  return *this;
}

void SDL2_Font::free() {
  if (font_ != nullptr && TTF_WasInit()) {
    TTF_CloseFont(font_);
    font_ = nullptr;
    path_.clear();
    size_ = 0;
    font_info_ = {};
  }
}

bool SDL2_Font::loadFont(const std::string& path, int size) {
  SDL2_Font::free();
  font_ = TTF_OpenFont(path.c_str(), size);
  if (font_ == nullptr) {
    ktp::logSDLError("TTF_OpenFont");
    return false;
  }
  path_ = path;
  size_ = size;
  SDL2_Font::queryFontInfo();
  return true;
}

This works, or at least I looks to me. But because it's my first time doing this stuff I just want to somebody take a look at it. What do you think? Is the way I implemented the methods/overloads correct?

Or maybe it's a bit overkill? Because I don't intend to copy/move/assign SDL2_Fonts and just for that destructor I had to do all those methods.

I don't need it. I just want my code to be as modern, elegant and maintainable as possible. — Alex CB
– Alex CB, Commented Feb 5, 2021 at 6:13
Can TTF_WasInit() actually be false after TTF_OpenFont()? If not (and given there's no other way to get a non-null font_), then there's no need to check for that, and the deleter can be much simpler. — Toby Speight
– Toby Speight, Commented Feb 5, 2021 at 7:47
@AlexCB I don't know about mantainable (code already looks crazy complex), but "elegant" and "modern" are just buzzwords. :) — user712092
– user712092, Commented Feb 11, 2021 at 17:04

Toby Speight · Accepted Answer · 2021-02-05 18:45:09Z

Because you

don't intend to copy/move/assign SDL2_Fonts

you could formalize that with

SDL2_Font(const SDL2_Font& other) = delete;
SDL2_Font& operator=(const SDL2_Font& other) = delete;

(the move versions of these operations won't be generated if these two are defined or deleted).

However, let's assume that one day we will want to be able to pass these fonts by value.

The code looks correct to my eye, but you could avoid having to do the memory management yourself if you use a smart pointer instead of a raw pointer for the font_ member. If you do that, you can follow the Rule of Zero (the best of these rules) and simply let the compiler create the appropriate copy operators and constructors - see worked code below.

~~I notice that free() can only ever be called from the destructor.~~ (Edit - I see it's also called from loadFont() but less obviously so, by being class-qualified. This paragraph still applies, though.) free() does a lot of "dead writes" most times it's called - i.e. making assignments to variables that can never be read. That's a waste of your time and mine! Possibly also of your processor's, though a decent compiler should be able to optimise most of that away.

Here's a simplified version using shared pointer (I'm assuming that copies don't each need their own font object, as we don't ever modify it):

#include <SDL.h>
#include <SDL_ttf.h>

#include <memory>
#include <string>

namespace ktp {
    void logSDLError(const char*);
}

/* header */
class SDL2_Font {
public:
    SDL2_Font() {}

    bool loadFont(const std::string& path, int size);
private:
    struct FontInfo {};
    void queryFontInfo();

    std::shared_ptr<const TTF_Font> font_{};
    FontInfo font_info_ {};
    std::string path_ {};
    int size_ = 0;
};

// look, no constructors or assignment operators!

bool SDL2_Font::loadFont(const std::string& path, int size)
{
    font_.reset(TTF_OpenFont(path.c_str(), size),
                &TTF_CloseFont);
    if (!font_) {
        ktp::logSDLError("TTF_OpenFont");
        font_info_ = {};
        path_ = {};
        size_ = 0;
        return false;
    }
    path_ = path;
    size_ = size;
    queryFontInfo();
    return true;
}

See how reset() takes care of releasing resources for us, and we don't need to write a destructor.

I tried this with a simple main():

int main()
{
    if (TTF_Init()) {
        std::cerr << "TTF_Init: " << TTF_GetError() << '\n';
        return 1;
    }

    {
        SDL2_Font f;
        f.loadFont("/usr/share/fonts/truetype/unifont/unifont.ttf", 14);
        f.loadFont("/usr/share/fonts/truetype/unifont/unifont.ttf", 16);
        // important - f goes out of scope before TTF_Quit()
    }

    TTF_Quit();
}

This runs cleanly under Valgrind:

==29172== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==29172== Command: ./255607
==29172== 
==29172== 
==29172== HEAP SUMMARY:
==29172==     in use at exit: 0 bytes in 0 blocks
==29172==   total heap usage: 155 allocs, 155 frees, 615,577 bytes allocated
==29172== 
==29172== All heap blocks were freed -- no leaks are possible

Headers are #include "./sdl2_log.h" #include <SDL.h> #include <SDL_ttf.h> #include <string>'. As you say, in another branch I'm trying the rule of zero approach, but I can't get the custom deleter for the smart pointer right, and reading the comments on this SO thread, it's looks that it causes a bit of overhead in all unique_ptr, isn't it? — Alex CB
– Alex CB, Commented Feb 5, 2021 at 6:05
I've tried Toby's solution in my code and it looks like it works: the fonts are correctly rendered. But I'm not sure what happens when the shared_ptr is deconstructed because this only happens when I terminate the program. If you need a main() to try you can grab mine from github (user lyquid, repo KUGE), but it's full of other SDL stuff. — Alex CB
– Alex CB, Commented Feb 5, 2021 at 11:30
@Alex I've updated to show a simple main() that exercises this class. That's a useful thing to do in any case - it's hard to properly test stuff if you can't isolate the pieces. — Toby Speight
– Toby Speight, Commented Feb 5, 2021 at 13:11
My bad, i read this on a phone and the line truncation in the scroll box made it seem like the deleter wasn't there. Sorry for the confusion. — Emily L.
– Emily L., Commented Feb 5, 2021 at 18:09
@Alex, to address your point about a custom deleter having overhead, that might or might not be true, depending on the cleverness of your compiler. But if it is, then I'd expect any overhead to be small relative to loading a font. And IMO well worth paying compared to the effort of maintaining pointer ownership by hand! — Toby Speight
– Toby Speight, Commented Feb 5, 2021 at 18:40

Stack Exchange Network

C++ wrapper for font opaque pointer

1 Answer 1

You must log in to answer this question.

Hot Network Questions

C++ wrapper for font opaque pointer

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions