From 570b40bdcd6fcd9bcb2145d82f3f552a6cbe2d6b Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Sat, 20 Jan 2024 22:18:35 +0100 Subject: [PATCH] Inspection: Polymorphic class with non-virtual public destructor --- tests/test_id3v2framefactory.cpp | 17 +++++++--- tests/test_mp4.cpp | 54 +++++++++++++++++++------------- 2 files changed, 45 insertions(+), 26 deletions(-) diff --git a/tests/test_id3v2framefactory.cpp b/tests/test_id3v2framefactory.cpp index 64afec1e..10de6352 100644 --- a/tests/test_id3v2framefactory.cpp +++ b/tests/test_id3v2framefactory.cpp @@ -85,6 +85,9 @@ namespace // Example for frame factory with support for CustomFrame. class CustomFrameFactory : public ID3v2::FrameFactory { public: + CustomFrameFactory(const CustomFrameFactory &) = delete; + CustomFrameFactory &operator=(const CustomFrameFactory &) = delete; + static CustomFrameFactory *instance() { return &factory; } ID3v2::Frame *createFrameForProperty( const String &key, const StringList &values) const override { if(key == "CUSTOM") { @@ -94,6 +97,8 @@ namespace } protected: + CustomFrameFactory() = default; + ~CustomFrameFactory() = default; ID3v2::Frame *createFrame(const ByteVector &data, ID3v2::Frame::Header *header, const ID3v2::Header *tagHeader) const override { if(header->frameID() == "CUST") { @@ -101,8 +106,12 @@ namespace } return ID3v2::FrameFactory::createFrame(data, header, tagHeader); } + + private: + static CustomFrameFactory factory; }; + CustomFrameFactory CustomFrameFactory::factory; } // namespace class TestId3v2FrameFactory : public CppUnit::TestFixture @@ -127,8 +136,6 @@ public: function getID3v2Tag, function stripAllTags) { - CustomFrameFactory factory; - { auto f = std::unique_ptr(createFileWithDefaultFactory(fileName)); CPPUNIT_ASSERT(f->isValid()); @@ -161,7 +168,7 @@ public: CPPUNIT_ASSERT(!dynamic_cast(frames.front())); } { - auto f = std::unique_ptr(createFileWithFactory(fileName, &factory)); + auto f = std::unique_ptr(createFileWithFactory(fileName, CustomFrameFactory::instance())); CPPUNIT_ASSERT(f->isValid()); CPPUNIT_ASSERT(hasID3v2Tag(*f)); ID3v2::Tag *tag = getID3v2Tag(*f); @@ -180,7 +187,7 @@ public: stripAllTags(*f); } { - auto f = std::unique_ptr(createFileWithFactory(fileName, &factory)); + auto f = std::unique_ptr(createFileWithFactory(fileName, CustomFrameFactory::instance())); CPPUNIT_ASSERT(f->isValid()); CPPUNIT_ASSERT(!hasID3v2Tag(*f)); ID3v2::Tag *tag = getID3v2Tag(*f); @@ -191,7 +198,7 @@ public: f->save(); } { - auto f = std::unique_ptr(createFileWithFactory(fileName, &factory)); + auto f = std::unique_ptr(createFileWithFactory(fileName, CustomFrameFactory::instance())); CPPUNIT_ASSERT(f->isValid()); CPPUNIT_ASSERT(hasID3v2Tag(*f)); ID3v2::Tag *tag = getID3v2Tag(*f); diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 359dab4c..c37e4e04 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -41,6 +41,36 @@ using namespace std; using namespace TagLib; +namespace +{ + + class CustomItemFactory : public MP4::ItemFactory { + public: + CustomItemFactory(const CustomItemFactory &) = delete; + CustomItemFactory &operator=(const CustomItemFactory &) = delete; + static CustomItemFactory *instance() { return &factory; } + protected: + CustomItemFactory() = default; + ~CustomItemFactory() = default; + NameHandlerMap nameHandlerMap() const override + { + return MP4::ItemFactory::nameHandlerMap() + .insert("tsti", ItemHandlerType::Int) + .insert("tstt", ItemHandlerType::Text); + } + + Map namePropertyMap() const override + { + return MP4::ItemFactory::namePropertyMap() + .insert("tsti", "TESTINTEGER"); + } + private: + static CustomItemFactory factory; + }; + + CustomItemFactory CustomItemFactory::factory; +} // namespace + class TestMP4 : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestMP4); @@ -739,24 +769,6 @@ public: void testItemFactory() { - class CustomItemFactory : public MP4::ItemFactory { - protected: - NameHandlerMap nameHandlerMap() const override - { - return MP4::ItemFactory::nameHandlerMap() - .insert("tsti", ItemHandlerType::Int) - .insert("tstt", ItemHandlerType::Text); - } - - Map namePropertyMap() const override - { - return MP4::ItemFactory::namePropertyMap() - .insert("tsti", "TESTINTEGER"); - } - }; - - CustomItemFactory factory; - ScopedFileCopy copy("no-tags", ".m4a"); { MP4::File f(copy.fileName().c_str()); @@ -784,7 +796,7 @@ public: } { MP4::File f(copy.fileName().c_str(), - true, MP4::Properties::Average, &factory); + true, MP4::Properties::Average, CustomItemFactory::instance()); CPPUNIT_ASSERT(f.isValid()); CPPUNIT_ASSERT(!f.hasMP4Tag()); MP4::Tag *tag = f.tag(); @@ -798,7 +810,7 @@ public: } { MP4::File f(copy.fileName().c_str(), - true, MP4::Properties::Average, &factory); + true, MP4::Properties::Average, CustomItemFactory::instance()); CPPUNIT_ASSERT(f.isValid()); CPPUNIT_ASSERT(f.hasMP4Tag()); MP4::Tag *tag = f.tag(); @@ -824,7 +836,7 @@ public: } { MP4::File f(copy.fileName().c_str(), - true, MP4::Properties::Average, &factory); + true, MP4::Properties::Average, CustomItemFactory::instance()); CPPUNIT_ASSERT(f.isValid()); CPPUNIT_ASSERT(f.hasMP4Tag()); MP4::Tag *tag = f.tag();