'Cannot Instantiate Abstract Class' when implementing GLFW Windows in C++

Question

I'm working on university coursework and attempting to build a window using GLFW. I've been following the documentation and video series we were given to follow (The Hazel Engine by The Cherno on Youtube), but have run into an issue with abstract classes.

I really struggle to understand both pointers and abstraction, so am really struggling to understand what I'm doing, but I believe it's attempting to call the inherited 'create' function from Window, to build a WinWindow (named as such because it's Windows OS specific), but I'm getting an error C2259 "'Engine::WinWindow' cannot instantiate abstract class" at line 9 of winWindow.cpp

The relevant code follows:

window.h

#pragma once

#include "graphicsContext.h"
#include <string>
#include <functional>

namespace Engine {

    class Event; // Be replaced

    struct WindowProperties
    {
        std::string m_title;
        unsigned int m_width;
        unsigned int m_height;
        bool m_isFullScreen;
        bool m_isVSync;

        WindowProperties(const std::string& title = "My Window", unsigned int width = 800, unsigned int height = 600, bool fullscreen = false) : m_title(title), m_width(width), m_height(height), m_isFullScreen(fullscreen) {}
    };

    class Window
    {
    public:
        using EventCallbackFn = std::function<void(Event&)>;
        virtual void init(const WindowProperties& properties) = 0;
        virtual void close() = 0;
        virtual ~Window() {};
        virtual void onUpdate(float timestep) = 0;
        virtual void onResize(unsigned int width, unsigned int height) = 0;
        virtual void setVSync(bool VSync) = 0;
        virtual void setEventCallback(const EventCallbackFn callback) = 0;
        virtual unsigned int getWidth() const = 0;
        virtual unsigned int getHeight() const = 0;
        virtual void* getNativeWindow() const = 0;
        virtual bool isFullScreenMode() const = 0;
        virtual bool isVSync() const = 0;

        static Window* create(const WindowProperties& properties = WindowProperties());
    protected:
        std::shared_ptr<GraphicsContext> m_context;
    };
}

winWindow.h

#pragma once

#include "windows/window.h"
#include <GLFW/glfw3.h>


namespace Engine {

    class WinWindow : public Window {

    public:
        WinWindow(const WindowProperties& properties);
        virtual ~WinWindow();

        void onUpdate();// override;

        inline unsigned int getWidth() const override { return m_data.width; }
        inline unsigned int getHeight() const override { return m_data.height; }

        inline void SetEventCallback(const EventCallbackFn& callback) override { m_data.eventCallback = callback; }
        void setVSync(bool enabled) override;
        bool isVSync() const override;

    private:
        virtual void init(const WindowProperties& properties);
        virtual void shutdown();
        GLFWwindow* m_window;
        struct windowData {
            std::string title;
            unsigned int width, height;
            bool vSync;
            EventCallbackFn eventCallback;
        };
        windowData m_data;

    };
}

winWindow.cpp

#include "engine_pch.h"
#include "Platform/win/winWindow.h"

namespace Engine {

    static bool GLFWinit = false;

    Window* Window::create(const WindowProperties& properties) {
        return new WinWindow(properties);
    }

    WinWindow::WinWindow(const WindowProperties& properties) {
        init(properties);
    }

    WinWindow::~WinWindow() {
        shutdown();
    }

    void WinWindow::init(const WindowProperties& properties) {
        m_data.title = properties.m_title;
        m_data.width = properties.m_width;
        m_data.height = properties.m_height;

        LOG_INFO("Window: {0} - ({1}, {2})", properties.m_title, properties.m_width, properties.m_height);

        if (!GLFWinit) {
            GLFWinit = true;
        }

        m_window = glfwCreateWindow((int)properties.m_width, (int)properties.m_height, m_data.title.c_str(), nullptr, nullptr);
        glfwMakeContextCurrent(m_window);
        glfwSetWindowUserPointer(m_window, &m_data);
        setVSync(true);
    }

    void WinWindow::shutdown() {
        glfwDestroyWindow(m_window);
    }

    void WinWindow::onUpdate() {
        glfwPollEvents();
        glfwSwapBuffers(m_window);
    }

    void WinWindow::setVSync(bool enabled) {
        if (enabled)
            glfwSwapInterval(1);
        else
            glfwSwapInterval(0);

        m_data.vSync = enabled;
    }

    bool WinWindow::isVSync() const {
        return m_data.vSync;
    }
}

I'm also getting a separate error C3668, which says "'Engine::WinWindow::SetEventCallback': method with override specifier 'override' did not override any base class methods". Although I could be wildly wrong, I believe this is just because it is as of right now, unused.

Any help with these issues will be greatly appreciated, but could you attempt to explain yourself as much as possible to help me understand what's happening and the decision making of what's going on, as I'm really struggling to follow all of this?

Answer 1

You must implement all the pure virtual functions from Window in WinWindow (those member functions with = 0 in the declaration). Otherwise WinWindow will be an abstract class (as Window is) and instances of abstract classes cannot be created, but you are trying to create an instance of type WinWindow with new WinWindow(properties) (which is what the error is telling you).

You have not overridden and implemented many of the classes e.g. close, onResize, etc.

You should not ignore the other error message. It means that you messed up when declaring the function in WinWindow.

The problem is that the virtual function in Window has this signature:

void setEventCallback(const EventCallbackFn callback)

But your supposed override has the signature:

void SetEventCallback(const EventCallbackFn& callback)

The function names are not the same and the parameter types are not the same either (one is a reference, the other isn't). The overriding functions must match the signature of the functions they are overriding.

Don't drop the override qualifier either. If you do that the error message will go away, but it will not actually solve the problem. If you are overriding a virtual function from a base class, always add override. The fact that it will give an error if you made a mistake is exactly why it is there in the first place. So add the override to onUpdate and make it match the virtual function it is supposed to override as well:

void onUpdate(float timestep) override;

---

Further notes unrelated to problem mentioned in the question:

---

Also note that you should almost never use new directly in modern C++. Use std::unique_ptr<Window> and std::make_unique<WinWindow> instead.

In fact, I don't see why you are returning Window* instead of WinWindow*, so make that std::unique_ptr<WinWindow> instead or even better return the WinWindow directly by-value (without any new or std::make_unique). You can still move it into an object under the control of a smart pointer at the call site if you need that for polymorphism.

But then if that is the case, you can also just use WinWindow's constructor directly instead of going through a create method. So create is kind of pointless then.

---

Also note that the inline keywords are completely redundant. You should remove them. They do nothing there except confuse the reader (for example: Do you know what they mean?). Functions defined directly in a class definition are inline automatically.

---

Also note that your class WinWindow is not following the rule of 0/3/5, because your class is managing a resource (the window handle) and you gave it a destructor that does something non-trivial (destroying the window), but you have not implemented a semantically correct copy constructor and copy assignment operator. Therefore your class will break if you ever happen to implicitly or explicitly copy it. This happens specifically when you use WinWindow's constructor directly to e.g. return by-value in create as I suggested above. So as long as you don't fix this you should stick with std::unique_ptr/std::make_unique.

The correct fix for this is in this case to implement the move constructor and move assignment operator with correct semantics. It makes no semantic sense to copy a Window (or at least you don't actually want to do that), but you can move a window handle from one object to another.

---

Admittedly these are relatively advanced subjects. This problem would not be present if you were just provided a wrapper class that wraps the C library (GLFW) in a proper C++ interface rather than doing this manually. I would expect your instructor to provide a wrapper class, at least around the window handle itself. Doing this correctly is difficult or impossible for a beginner if you haven't learned about move semantics and resource management yet.

I suggest you learn from a good introductory book that covers all of this before you try yourself at something so complicated.