-1
\$\begingroup\$

I have a very class heavy approach to writing C++ code, which I don't think is necessarily wrong. However, I often use classes without knowing if I actually need them, which I feel leads to making poor/unnecessary design choices. I think this may be a problem. I know there isn’t a definitive right answer to this, but perhaps there are guidelines that could help nudge me in the right direction when it comes to making design decisions?

To use as an example here is an asset system I tried to make as part of my game engine library learning project.

class IAssetLoader {
public:
    virtual ~IAssetLoader() = default;
    virtual std::unique_ptr<std::any> load(const AssetMetadata& metadata) = 0;
};

class MeshLoader : public IAssetLoader {
public:
    MeshLoader(IGraphicsDevice* graphicsDevice);
    std::unique_ptr<std::any> load(const AssetMetadata& metadata) override;

private:
    IGraphicsDevice* m_graphicsDevice;
};

class TextureLoader : public IAssetLoader {
public:
    TextureLoader(IGraphicsDevice* graphicsDevice);
    std::unique_ptr<std::any> load(const AssetMetadata& metadata) override;

private:
    IGraphicsDevice* m_graphicsDevice;
};

This made sense to me because a MeshLoader and TextureLoader both load assets, therefore are types of an asset loader. However, from what I understand about inheritance and polymorphism, they aren't really interchangeable. Where I use a mesh loader I'm probably not going to use a texture loader and vice versa. There isn't any shared logic/state as of yet and the state they do have (the graphics device) could probably just be passed as a method parameter instead. I think the only advantage I get out of this is being able to store them into a single container std::unordered_map<AssetType, std::unique_ptr<IAssetLoader>> loaders; inside of an AssetManager class.

\$\endgroup\$
3
  • \$\begingroup\$ It is very hard to say whether this is a good or a bad design without seeing some more concrete code. See this reason for why. I would personally KISS, and I suspect YAGNI. \$\endgroup\$ Commented May 12 at 5:23
  • \$\begingroup\$ The current title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$ Commented May 12 at 9:17
  • \$\begingroup\$ std::unique_ptr<std::any> is a very smelly type, which makes IAssetLoader a smelly type too. If you have an IAssetLoader, and no other context, you can't do anything useful. \$\endgroup\$ Commented May 14 at 16:42

1 Answer 1

Reset to default
3
\$\begingroup\$

Based on my experience and what you've said, I think the case for inheritance here is extremely weak, even at best.

Once you've loaded them, you use textures and meshes entirely differently. When you need to do something with a texture, you know you're not looking for a mesh (and vice versa). As such, even if you can store the asset loaders into a single collection, you almost certainly don't even want to do the same with the assets they load. So you probably haven't gained much (if anything) by having a single collection with loaders for different types of assets.

Going a step further, it's not clear to me that it makes a whole lot of sense for an asset loader to be a class at all (or at least not one on which you really focus). My immediate reaction would be that the class should be the asset itself, and it should have probably have a load member function.

class Texture {
    SomeType data;
public:
    bool load(AssetMetadata const &AssetSpecification, IGraphicsDevice &device);
};

class Mesh {
    SomeOtherType data;
public:
    bool load(AssetMetadata const &AssetSpecification, IGraphicsDevice &device);
};
\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.