使用std：：向量存储纹理

Question

我需要编写一个存储TextureManager对象的Texture类；问题是我使用std::map<Texture *, const char*>映射来使用键存储Texture对象。

由于性能问题，通过映射搜索元素并不理想(我正在编写C++库，因此性能非常重要)。

我有过使用std::vector<Texture*>的想法。在这种情况下，我需要使用unsigned int。但是经过几次尝试，我仍然无法实现它。

下面是.h：

class CTextureManager
{
public:
    CTextureManager(SDL_Renderer *pRenderer);
    void SetRenderer(SDL_Renderer *pRenderer);

    ~CTextureManager();

    // Load File function
    CTexture *LoadFromFile(std::string name, std::string filename);

    bool UnLoad(std::string name);

    CTexture *GetTexture(std::string name);

    void ClearAll();


protected:
    std::map<std::string, CTexture *> m_mapTexture;
    SDL_Renderer *m_pRenderer;

};

.cpp：

CTextureManager::CTextureManager(SDL_Renderer *pRenderer)
{
    m_pRenderer = pRenderer;
}

CTextureManager::~CTextureManager()
{
    ClearAll();

    if (m_pFont != nullptr)
    {
        TTF_CloseFont(m_pFont);
    }
}

void CTextureManager::ClearAll()
{
    for (std::map<std::string, CTexture *>::iterator it = m_mapTexture.begin(); it != m_mapTexture.end(); ++it)
    {
        delete (it->second);
        (it->second) = nullptr;
    }
    m_mapTexture.clear();
}

CTexture * CTextureManager::GetTexture(std::string name)
{
    return m_mapTexture[name];
}

bool CTextureManager::UnLoad(std::string name)
{
    CTexture *pTempTexture = m_mapTexture[name];

    if (!pTempTexture)
        return false;

    delete pTempTexture;
    pTempTexture = nullptr;
    m_mapTexture.erase(name);
    return true;
}


CTexture * CTextureManager::LoadFromFile(std::string name, std::string filename)
{
    CTexture *pTexture = m_mapTexture[name];

    if (pTexture == nullptr)
    {
        pTexture = new CTexture();
        pTexture->SetRenderer(m_pRenderer);

        m_mapTexture[name] = pTexture;
    }

    if (!pTexture->LoadFromFile(filename))
        return nullptr;

    return pTexture;
}

void CTextureManager::SetRenderer(SDL_Renderer *pRenderer)
{
    m_pRenderer = pRenderer;
}

Answer 1

接口设计很差：

• 你可以在这里通过nullptr吗？
• 对象是否拥有指针的所有权？CTextureManager(SDL_Renderer *pRenderer)；
• 你可以更改渲染器吗？
• 你可以用nullptr替换它吗？
• 所有权是否传递给CTextureManager对象？void SetRenderer(SDL_Renderer *pRenderer)；
• 按值传递name和filename。这导致了一份副本。这是低效和不需要的。通过const参考。
• 返回的值：
• 允许是nullptr吗？
• 指针是谁的？CTexture *LoadFromFile(std::string，std::string )；
• 另一个通过价值传递。
• 返回值表示什么？bool UnLoad(std::string name)；
• 价值再过一次。
• 谁拥有归还的物品？CTexture *name(std：：string name)；

你在存储指针。

    std::map<std::string, CTexture *> m_mapTexture;
    SDL_Renderer *m_pRenderer;

这意味着您的类同时执行资源管理和业务逻辑操作。一个类应该做一个或另一个(搜索关注点的分离)。将资源管理拆分到它自己的类中。然后在纹理管理器中使用这个。

所有权语义.

您的主要问题是您没有正确地指示对象的所有权语义。

所有权是关于谁负责删除动态分配的对象，并且是C++的核心原则之一(在内存管理方面将C++提高到C以上)。如果您在接口中没有很好地定义这些语义，那么人们就必须深入了解实现，以了解所有权是如何工作的，这样他们才能正确地编写代码。

如果人们正在深入您的代码来理解所有权语义，那么他们现在取决于代码的实现细节。这使得您的代码对于将来的任何更改都非常脆弱，并将使用类的任何代码紧密耦合在一起。

• 通过引用返回，以表明所有权没有被转移。
• 通过引用表示您不接受所有权。
• 使用智能指针传递唯一/共享的所有权对象。
• 不要通过指针传递公共接口
  • 可以在内部使用，但它不应该是您与其他代码的公共接口的一部分。

更喜欢使用构造函数初始化列表.

TextureManager::CTextureManager(SDL_Renderer *pRenderer)
{
    m_pRenderer = pRenderer;
}

// When you have members that have constructors.
// These will be called before the body of the constructor is
// entered.
//
// By initializing in the body of the code you
// are first constructing the object into the default state
// the updating that state. It is better to call the constructor
// directly with the appropriate parameters.
//
// Also reference must be done in the initialization list.
// So prefer to do all initialization in the initialization list
// for consistency.
TextureManager::CTextureManager(SDL_Renderer *pRenderer)
    : m_pRenderer(pRenderer)
{}

析构函数

你的破坏者不是virtual。

CTextureManager::~CTextureManager()

这意味着您不期望有任何子类型的对象。这很好。但你应该记住这一点。

误差

    if (m_pFont != nullptr)
    {
        TTF_CloseFont(m_pFont);
    }

m_pFont不是这个类的成员。因此，我希望这不会编译。

清除所有

    for (std::map<std::string, CTexture *>::iterator it = m_mapTexture.begin(); it != m_mapTexture.end(); ++it)

   // Prefer to use `auto` for types you don't care about.
   // Also prefer to use the std::begin() and std::end() rather than
   // calling the methods directly. This will allow you to change the
   // container type in the future without having to change the code.
    for (auto it = std::begin(m_mapTexture); it != std::end(m_mapTexture); ++it)

   // Even better would be to use the new range based for
   for(auto& value: m_mapTexture)

   // Or potentially an algorithm:
   std::for_each(td::begin(m_mapTexture), std::end(m_mapTexture), /* ACTION */);

没有必要将其设置为nullptr：

        (it->second) = nullptr;

当你完成的时候，你清除了容器。因此，这些价值不再存在。

GetTexture

在std::map上，如果不存在值，operator[]将向容器中插入一个值。我不认为这是你真正想要的。

    return m_mapTexture[name];

UnLoad

再来一次insert。

    CTexture *pTempTexture = m_mapTexture[name];

没必要做这个测试。

    if (!pTempTexture)
        return false;

删除nullptr是可以的。

    delete pTempTexture;

这没有任何用处(因为它即将超出范围)。

    pTempTexture = nullptr;

由于插入了名称，所以可能希望始终删除它(即使指针为null)。

    m_mapTexture.erase(name);

只需在这里返回指针：

    return true;

指针将被转换为正确的bool。如果你什么也没发现，那就是false。如果你发现了什么，那就是true。

LoadFromFile

因此，您的对象始终是一个CTexture，而不是从它派生的任何东西。

        pTexture = new CTexture();

因为您的纹理是单一类型，所以我不认为需要在容器中存储指针。直接存储对象即可。

似乎纹理管理器拥有渲染器的唯一原因就是进行这个调用。看上去很奇怪。为什么不将呈现器传递给LoadFromFile()方法呢？

        pTexture->SetRenderer(m_pRenderer);

您刚刚动态地分配并在您的结构中放置了一个CTecture。您现在可以返回一个nullptr。但是，对GetTexture()的调用将返回先前创建的纹理对象(加载失败)。在加载失败时，您可能希望从结构中删除此对象。

    if (!pTexture->LoadFromFile(filename))
        return nullptr;

，我就是这样做的：

// I am guessing a bit because I have not read the SDL documentation.
struct SLDTextureDeleter
{
    void operator()(SDL_Texture* p){SDL_DestroyTexture(p);}
};
class CTextureManager;
class CTexture
{
        // Constructor is private so it can only be used by CTextureManager
        friend class CTextureManager;
        CTexture(std::string const& filename, CRenderer& renderer)
        {
            // Create Texture and load from File.
            // Failure to load leaves the texture pointer as nullptr.
            // The TextureManager tests this by calling ok() to validate
            // that the texture was loaded correctly.

        }
        bool ok() const {return texture.get();}
    public:
        // Allow texture moving
        CTexture(CTexture&& move)
            : texture(std::move(move.texture))
        {}
        CTexture& operator=(CTexture&& move)
        {
            using std::swap;
            swap(texture, move.texture);
        }
        // But disable Copy (as it is manager by CTextureManager)
        CTexture(CTexture const&)            = delete;
        CTexture& operator=(CTexture const&) = delete;
    private:
        std::auto_ptr<SDL_Texture, SLDTextureDeleter>   texture;
};
class CTextureManager
{
  public:
    bool      loadFromFile(std::string const& name, std::string const& filename, CRenderer& renderer);
    bool      hasTexture(std::string const& name) const;    
    CTexture& getTexture(std::string const& name) const;

    void clearAll();
    void unLoad(std::string const& name);    

  private:
    std::map<std::string, CTexture> mapTextures;
};

bool CTextureManager::loadFromFile(std::string const& name, std::string const& filename, CRenderer& renderer)
{
    CTexture  tmp(filename, renderer);
    bool result = tmp.ok();
    if (result)
    {
        mapTextures.emplace(name, std::move(tmp));
    }
    return result;
}
bool CTextureManager::hasTexture(std::string const& name) const
{
    auto find =   mapTextures.find(name);
    return find != mapTextures.end();
}
CTexture& CTextureManager::getTexture(std::string const& name) const;
{
    // Note: UB if loadFromFile() failed to return true.
    //          or if hasTexture() returned false.
    //          user is supposed to check before use.
    auto find =   mapTextures.find(name);
    return find->second;
}

void CTextureManager::clearAll()
{
    mapTextures.clear();
}
void CTextureManager::unLoad(std::string const& name)
{
    mapTextures.erase(name);
}

Answer 2

我知道这不是真正的问题，但为了进行更多的优化，我认为您应该传递成员函数的一些变量，而不是副本~

PS:我喜欢你的编码风格！