Additional refactoring

- apply factory method pattern
- make const member function whenever possible
- use member-initializer instead of constructor-initialization
- remove some duplicated codes
- add some empty lines for enhancing the readability
- remove namespace which is already using

Change-Id: I5ee8d1e4216803011de158d69a7e2a9b88f51438

diff --git a/src/metadata_editor.cpp b/src/metadata_editor.cpp
index 81f57e727f5dba61e22894561db7278e5e3d3573..42dac806b73d8e07447b1f35e05fa7b1301be0dc 100755 (executable)
--- a/src/metadata_editor.cpp
+++ b/src/metadata_editor.cpp
@@ -48,39 +48,44 @@ static String __get_file_ext(const char *file_path)
        return "";
 }
 
-static const String __get_picture_type(const char *path)
+static const char * __get_picture_type(const char *path)
 {
        String ext = __get_file_ext(path);
-       if (ext == "JPG" || ext == "JPEG") {
+       if (ext == "JPG" || ext == "JPEG")
                return MIME_TYPE_JPEG;
-       } else if (ext == "PNG") {
+       if (ext == "PNG")
                return MIME_TYPE_PNG;
-       }
+
        return "";
 }
 
 class PictureFrame
 {
 public:
-       explicit PictureFrame(const char *path) : stream(path, true) {}
-       ~PictureFrame() = default;
+       explicit PictureFrame(const char *path)
+               : stream(path, true) {}
+       virtual ~PictureFrame() = default;
+
        ByteVector data() {
                return stream.readBlock(stream.length());
        }
+
        String mime() const {
                return __get_picture_type(stream.name());
        }
-       bool opened() {
+
+       bool opened() const {
                return stream.isOpen();
        }
-       MP4::CoverArt::Format mp4format() {
+
+       MP4::CoverArt::Format mp4format() const {
                if (mime() == MIME_TYPE_JPEG)
                        return MP4::CoverArt::JPEG;
-               else if (mime() == MIME_TYPE_PNG)
+               if (mime() == MIME_TYPE_PNG)
                        return MP4::CoverArt::PNG;
-               else
-                       return MP4::CoverArt::Unknown;
+               return MP4::CoverArt::Unknown;
        }
+
 private:
        FileStream stream;
 };
@@ -89,7 +94,8 @@ template <class T>
 static bool __is_valid_index(const List<T>& lst, int index)
 {
        ME_RETVM_IF(lst.isEmpty(), false, "No picture");
-       ME_RETVM_IF((index < 0) || (lst.size() <= static_cast<uint>(index)), false, "Out of range:size[%d] index[%d]", lst.size(), index);
+       ME_RETVM_IF((index < 0) || (lst.size() <= static_cast<uint>(index)),
+                               false, "Out of range:size[%d] index[%d]", lst.size(), index);
        return true;
 }
 
@@ -134,6 +140,7 @@ static int __append_APIC(ID3v2::Tag *tag, PictureFrame& pic)
        auto pictureFrame = new ID3v2::AttachedPictureFrame();
        pictureFrame->setPicture(pic.data());
        pictureFrame->setMimeType(pic.mime());
+
        tag->addFrame(pictureFrame);
 
        return METADATA_EDITOR_ERROR_NONE;
@@ -146,6 +153,7 @@ static int __append_ogg_picture(Ogg::XiphComment *xtag, PictureFrame& pic)
        auto frontCover = new FLAC::Picture;
        frontCover->setData(pic.data());
        frontCover->setMimeType(pic.mime());
+
        xtag->addPicture(frontCover);
 
        return METADATA_EDITOR_ERROR_NONE;
@@ -172,6 +180,7 @@ static int __remove_ogg_picture(Ogg::XiphComment *xtag, int index)
 
        List<FLAC::Picture*>::Iterator it = lst.begin();
        std::advance(it, index);
+
        xtag->removePicture(*it, true);
 
        return METADATA_EDITOR_ERROR_NONE;
@@ -181,6 +190,7 @@ class IAlbumArt
 {
 public:
        virtual ~IAlbumArt() = default;
+
        virtual int append(PictureFrame &pic) = 0;
        virtual int remove(int index) = 0;
        virtual int read(int index, void **picture, int *size, char **mime_type) = 0;
@@ -189,33 +199,35 @@ public:
 
 class Mp3AlbumArt : public IAlbumArt {
 public:
-       explicit Mp3AlbumArt(FileRef *fileref) {
-               file = dynamic_cast<MPEG::File *>(fileref->file());
-       }
-       ~Mp3AlbumArt() override {};
+       explicit Mp3AlbumArt(FileRef *fileref)
+               : file(dynamic_cast<MPEG::File *>(fileref->file())) {}
+       ~Mp3AlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                return __append_APIC(file->ID3v2Tag(true), pic);
        }
+
        int remove(int index) override {
                return __remove_APIC(file->ID3v2Tag(), index);
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                return __get_APIC(file->ID3v2Tag(), index, picture, size, mime_type);
        }
+
        uint count() override {
                return file->ID3v2Tag() ? file->ID3v2Tag()->frameListMap()["APIC"].size() : 0;
        }
+
 private:
        MPEG::File *file;
 };
 
 class Mp4AlbumArt : public IAlbumArt {
 public:
-       explicit Mp4AlbumArt(FileRef *fileref) {
-               file = dynamic_cast<MP4::File *>(fileref->file());
-       }
-       ~Mp4AlbumArt() override {};
+       explicit Mp4AlbumArt(FileRef *fileref)
+               : file(dynamic_cast<MP4::File *>(fileref->file())) {}
+       ~Mp4AlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                MP4::CoverArtList lst;
@@ -230,6 +242,7 @@ public:
 
                return METADATA_EDITOR_ERROR_NONE;
        }
+
        int remove(int index) override {
                ME_RETVM_IF(!(file->tag()->contains("covr")), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "No picture");
                auto lst = file->tag()->item("covr").toCoverArtList();
@@ -243,6 +256,7 @@ public:
 
                return METADATA_EDITOR_ERROR_NONE;
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                ME_RETVM_IF(!(file->tag()->contains("covr")), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "No picture");
                auto lst = file->tag()->item("covr").toCoverArtList();
@@ -261,19 +275,20 @@ public:
 
                return METADATA_EDITOR_ERROR_NONE;
        }
+
        uint count() override {
                return (file->tag() && file->tag()->contains("covr")) ? file->tag()->item("covr").toCoverArtList().size() : 0;
        }
+
 private:
        MP4::File *file;
 };
 
 class FlacAlbumArt : public IAlbumArt {
 public:
-       explicit FlacAlbumArt(FileRef *fileref) {
-               file = dynamic_cast<FLAC::File *>(fileref->file());
-       }
-       ~FlacAlbumArt() override {};
+       explicit FlacAlbumArt(FileRef *fileref)
+               : file(dynamic_cast<FLAC::File *>(fileref->file())) {}
+       ~FlacAlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                auto frontCover = new FLAC::Picture;
@@ -282,6 +297,7 @@ public:
                file->addPicture(frontCover);
                return METADATA_EDITOR_ERROR_NONE;
        }
+
        int remove(int index) override {
                auto lst = file->pictureList();
                ME_RETV_IF(!__is_valid_index(lst, index), METADATA_EDITOR_ERROR_INVALID_PARAMETER);
@@ -289,87 +305,122 @@ public:
                file->removePicture(lst[index], true);
                return METADATA_EDITOR_ERROR_NONE;
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                return __get_flac_picture(file->pictureList(), index, picture, size, mime_type);
        }
+
        uint count() override {
                return file->pictureList().size();
        }
+
 private:
        FLAC::File *file;
 };
 
 class OggVorbisAlbumArt : public IAlbumArt {
 public:
-       explicit OggVorbisAlbumArt(FileRef *fileref) {
-               file = dynamic_cast<Ogg::Vorbis::File *>(fileref->file());
-       }
-       ~OggVorbisAlbumArt() override {};
+       explicit OggVorbisAlbumArt(FileRef *fileref)
+               : file(dynamic_cast<Ogg::Vorbis::File *>(fileref->file())) {}
+       ~OggVorbisAlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                return __append_ogg_picture(file->tag(), pic);
        }
+
        int remove(int index) override {
                return __remove_ogg_picture(file->tag(), index);
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                ME_RETVM_IF(!file->tag(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid XiphComment");
                return __get_flac_picture(file->tag()->pictureList(), index, picture, size, mime_type);
        }
+
        uint count() override {
                return file->tag() ? file->tag()->pictureList().size() : 0;
        }
+
 private:
        Ogg::Vorbis::File *file;
 };
 
 class OggFlacAlbumArt : public IAlbumArt {
 public:
-       explicit OggFlacAlbumArt(FileRef *fileref) {
-               file = dynamic_cast<Ogg::FLAC::File *>(fileref->file());
-       }
-       ~OggFlacAlbumArt() override {};
+       explicit OggFlacAlbumArt(FileRef *fileref)
+               : file(dynamic_cast<Ogg::FLAC::File *>(fileref->file())) {}
+       ~OggFlacAlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                return __append_ogg_picture(file->tag(), pic);
        }
+
        int remove(int index) override {
                return __remove_ogg_picture(file->tag(), index);
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                ME_RETVM_IF(!file->tag(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid XiphComment");
                return __get_flac_picture(file->tag()->pictureList(), index, picture, size, mime_type);
        }
+
        uint count() override {
                return file->tag() ? file->tag()->pictureList().size() : 0;
        }
+
 private:
        Ogg::FLAC::File *file;
 };
 
 class WavAlbumArt : public IAlbumArt {
 public:
-       explicit WavAlbumArt(FileRef *fileref) {
-               file = dynamic_cast<RIFF::WAV::File *>(fileref->file());
-       }
-       ~WavAlbumArt() override {};
+       explicit WavAlbumArt(FileRef *fileref)
+               : file(dynamic_cast<RIFF::WAV::File *>(fileref->file())) {}
+       ~WavAlbumArt() override = default;
 
        int append(PictureFrame &pic) override {
                return __append_APIC(file->tag(), pic);
        }
+
        int remove(int index) override {
                return __remove_APIC(file->tag(), index);
        }
+
        int read(int index, void **picture, int *size, char **mime_type) override {
                return __get_APIC(file->tag(), index, picture, size, mime_type);
        }
+
        uint count() override {
                return file->tag() ? file->tag()->frameListMap()["APIC"].size() : 0;
        }
+
 private:
        RIFF::WAV::File *file;
 };
 
+class AlbumArtFactory {
+public:
+       static IAlbumArt* create(FileRef *fref) {
+               if (fref == nullptr)
+                       return nullptr;
+
+               if (dynamic_cast<MPEG::File *>(fref->file()))
+                       return new Mp3AlbumArt(fref);
+               if (dynamic_cast<MP4::File *>(fref->file()))
+                       return new Mp4AlbumArt(fref);
+               if (dynamic_cast<FLAC::File *>(fref->file()))
+                       return new FlacAlbumArt(fref);
+               if (dynamic_cast<RIFF::WAV::File *>(fref->file()))
+                       return new WavAlbumArt(fref);
+               if (dynamic_cast<Ogg::Vorbis::File *>(fref->file()))
+                       return new OggVorbisAlbumArt(fref);
+               if (dynamic_cast<Ogg::FLAC::File *>(fref->file()))
+                       return new OggFlacAlbumArt(fref);
+
+               return nullptr;
+       }
+};
+
 typedef struct _metadata_editor_s {
        FileRef *fref;
        IAlbumArt *ifart;
@@ -503,18 +554,7 @@ extern "C" int metadata_editor_set_path(metadata_editor_h metadata, const char *
                return METADATA_EDITOR_ERROR_PERMISSION_DENIED;
        }
 
-       if (dynamic_cast<MPEG::File *>(_metadata->fref->file()))
-               _metadata->ifart = new Mp3AlbumArt(_metadata->fref);
-       else if (dynamic_cast<MP4::File *>(_metadata->fref->file()))
-               _metadata->ifart = new Mp4AlbumArt(_metadata->fref);
-       else if (dynamic_cast<FLAC::File *>(_metadata->fref->file()))
-               _metadata->ifart = new FlacAlbumArt(_metadata->fref);
-       else if (dynamic_cast<RIFF::WAV::File *>(_metadata->fref->file()))
-               _metadata->ifart = new WavAlbumArt(_metadata->fref);
-       else if (dynamic_cast<Ogg::Vorbis::File *>(_metadata->fref->file()))
-               _metadata->ifart = new OggVorbisAlbumArt(_metadata->fref);
-       else if (dynamic_cast<Ogg::FLAC::File *>(_metadata->fref->file()))
-               _metadata->ifart = new OggFlacAlbumArt(_metadata->fref);
+       _metadata->ifart = AlbumArtFactory::create(_metadata->fref);
 
        return METADATA_EDITOR_ERROR_NONE;
 }
@@ -527,6 +567,7 @@ extern "C" int metadata_editor_append_picture(metadata_editor_h metadata, const
 
        ret = __check_file_validity(path);
        ME_RETVM_IF(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid path");
+
        PictureFrame pic(path);
        ME_RETVM_IF(!pic.opened(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid path");
 
@@ -552,82 +593,59 @@ extern "C" int metadata_editor_get_picture(metadata_editor_h metadata, int index
        return _metadata->ifart->read(index, picture, size, mime_type);
 }
 
-extern "C" int metadata_editor_set_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, const char *value)
+static const char * __get_attr_str(metadata_editor_attr_e attribute)
 {
-       auto _metadata = static_cast<metadata_editor_s*>(metadata);
-       ME_RETV_IF(__is_valid_handle(_metadata, true), METADATA_EDITOR_ERROR_INVALID_PARAMETER);
-
        switch (attribute) {
        case METADATA_EDITOR_ATTR_ARTIST:
-               return __set_to_property_map(_metadata->fref->file(), String("ARTIST"), value);
+               return "ARTIST";
        case METADATA_EDITOR_ATTR_TITLE:
-               return __set_to_property_map(_metadata->fref->file(), String("TITLE"), value);
+               return "TITLE";
        case METADATA_EDITOR_ATTR_ALBUM:
-               return __set_to_property_map(_metadata->fref->file(), String("ALBUM"), value);
+               return "ALBUM";
        case METADATA_EDITOR_ATTR_GENRE:
-               return __set_to_property_map(_metadata->fref->file(), String("GENRE"), value);
+               return "GENRE";
        case METADATA_EDITOR_ATTR_AUTHOR:
-               return __set_to_property_map(_metadata->fref->file(), String("COMPOSER"), value);
+               return "COMPOSER";
        case METADATA_EDITOR_ATTR_COPYRIGHT:
-               return __set_to_property_map(_metadata->fref->file(), String("COPYRIGHT"), value);
+               return "COPYRIGHT";
        case METADATA_EDITOR_ATTR_DATE:
-               return __set_to_property_map(_metadata->fref->file(), String("DATE"), value);
+               return "DATE";
        case METADATA_EDITOR_ATTR_DESCRIPTION:
-               return __set_to_property_map(_metadata->fref->file(), String("DESCRIPTION"), value);
+               return "DESCRIPTION";
        case METADATA_EDITOR_ATTR_COMMENT:
-               return __set_to_property_map(_metadata->fref->file(), String("COMMENT"), value);
+               return "COMMENT";
        case METADATA_EDITOR_ATTR_TRACK_NUM:
-               return __set_to_property_map(_metadata->fref->file(), String("TRACKNUMBER"), value);
+               return "TRACKNUMBER";
        case METADATA_EDITOR_ATTR_CONDUCTOR:
-               return __set_to_property_map(_metadata->fref->file(), String("CONDUCTOR"), value);
+               return "CONDUCTOR";
        case METADATA_EDITOR_ATTR_UNSYNCLYRICS:
-               return __set_to_property_map(_metadata->fref->file(), String("LYRICS"), value);
+               return "LYRICS";
        default:
-               ME_ERR("Invalid attribute");
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
+               return "";
        }
 }
 
+extern "C" int metadata_editor_set_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, const char *value)
+{
+       auto _metadata = static_cast<metadata_editor_s*>(metadata);
+       ME_RETV_IF(__is_valid_handle(_metadata, true), METADATA_EDITOR_ERROR_INVALID_PARAMETER);
+
+       return __set_to_property_map(_metadata->fref->file(), __get_attr_str(attribute), value);
+}
+
 extern "C" int metadata_editor_get_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, char **value)
 {
        auto _metadata = static_cast<metadata_editor_s*>(metadata);
        ME_RETV_IF(__is_valid_handle(_metadata, false), METADATA_EDITOR_ERROR_INVALID_PARAMETER);
        ME_RETVM_IF(!value, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid value");
 
-       PropertyMap tags = _metadata->fref->file()->properties();
-
-       switch (attribute) {
-       case METADATA_EDITOR_ATTR_ARTIST:
-               return __get_from_property_map(tags, String("ARTIST"), value);
-       case METADATA_EDITOR_ATTR_TITLE:
-               return __get_from_property_map(tags, String("TITLE"), value);
-       case METADATA_EDITOR_ATTR_ALBUM:
-               return __get_from_property_map(tags, String("ALBUM"), value);
-       case METADATA_EDITOR_ATTR_GENRE:
-               return __get_from_property_map(tags, String("GENRE"), value);
-       case METADATA_EDITOR_ATTR_AUTHOR:
-               return __get_from_property_map(tags, String("COMPOSER"), value);
-       case METADATA_EDITOR_ATTR_COPYRIGHT:
-               return __get_from_property_map(tags, String("COPYRIGHT"), value);
-       case METADATA_EDITOR_ATTR_DATE:
-               return __get_from_property_map(tags, String("DATE"), value);
-       case METADATA_EDITOR_ATTR_DESCRIPTION:
-               return __get_from_property_map(tags, String("DESCRIPTION"), value);
-       case METADATA_EDITOR_ATTR_COMMENT:
-               return __get_from_property_map(tags, String("COMMENT"), value);
-       case METADATA_EDITOR_ATTR_TRACK_NUM:
-               return __get_from_property_map(tags, String("TRACKNUMBER"), value);
-       case METADATA_EDITOR_ATTR_CONDUCTOR:
-               return __get_from_property_map(tags, String("CONDUCTOR"), value);
-       case METADATA_EDITOR_ATTR_UNSYNCLYRICS:
-               return __get_from_property_map(tags, String("LYRICS"), value);
-       case METADATA_EDITOR_ATTR_PICTURE_NUM:
+       /* exceptional case */
+       if (attribute == METADATA_EDITOR_ATTR_PICTURE_NUM) {
                *value = g_strdup_printf("%u", _metadata->ifart->count());
                return METADATA_EDITOR_ERROR_NONE;
-       default:
-               ME_ERR("Invalid attribute [%d]", attribute);
-               return METADATA_EDITOR_ERROR_INVALID_PARAMETER;
        }
+
+       return __get_from_property_map(_metadata->fref->file()->properties(), __get_attr_str(attribute), value);
 }
 
 extern "C" int metadata_editor_update_metadata(metadata_editor_h metadata)