Упрощение применения небольших водяных знаков

Question 1

Я недавно начал работать с C ++ и работаю над приложением, которое может помечать водяными знаками сразу целые каталоги изображений.

В настоящее время вы можете вызвать программу с аргументом целевого каталога, в котором хранятся изображения, и аргументом, указывающим путь, к которому использовать изображение водяного знака.

Пока это работает, но я думаю, что мой код немного запутан. Я действительно не знаю, как сделать его более читаемым и чистым, поэтому прошу вашей помощи.

Я был бы очень признателен, если бы люди могли посмотреть мой код и просмотреть его. Любые отзывы приветствуются.

Ссылка на мой репозиторий GitHub: репозиторий github

Main.CPP:

#include <iostream>
#include "FileSystem.h"
#include "Watermarker.h"

int main(int arc, const char** argv)
{
    // Get the given argument for the target directory
    std::string targetDirectory = argv[1];

    // Get the given argument for the watermark file
    std::string watermarkPath = argv[2];

    FileSystem fileSystem;

    // Check if the target directory is valid
    if (!fileSystem.checkDirectory(targetDirectory)) { return 1; }

    // Create an output directory where the new watermarked images will be stored
    std::string path = targetDirectory + "\watermarked_images";
    fileSystem.createDirectory(path);

    Watermarker marker;

    // Mark all the images in the target directory with the watermarkPath image
    marker.mark(targetDirectory, watermarkPath);
}

FileSystem.h

#pragma once

#include <string>
#include <vector>
#include <boost/filesystem.hpp>

class FileSystem
{
private:
    std::vector<std::string> validExtensions = {
        ".png",
        ".jpg",
        ".tiff",
        ".jpeg"
    };

public:
    void createDirectory(std::string& directory);
    bool checkDirectory(std::string& directory);
    bool checkFile(boost::filesystem::path& filePath);
};

FileSystem.CPP

#include "FileSystem.h"
#include <iostream>
#include <boost/algorithm/string.hpp>

void FileSystem::createDirectory(std::string& directory)
{
    boost::filesystem::create_directory(directory);
}

bool FileSystem::checkDirectory(std::string& directory)
{
    if (boost::filesystem::exists(directory))
    {
        if (!boost::filesystem::is_directory(directory))
        {
            std::cout << "The given path is not a directory.n";
            return false;
        }
    }
    else
    {
        std::cout << directory << " does not existn";
        return false;
    }

    return true;
}

bool FileSystem::checkFile(boost::filesystem::path& filePath)
{
    // Check if the file is a regular file
    if (!boost::filesystem::is_regular_file(filePath)) { return false; }

    // Get the file extension in lower case
    std::string fileExtension = filePath.extension().string();
    boost::to_lower(fileExtension);

    // Check if the file extension matches one of the valid ones
    if (std::find(validExtensions.begin(), validExtensions.end(), fileExtension) != validExtensions.end())
    {
        return true;
    }
    else
    {
        return false;
    }
}

Watermarker.h

#pragma once

#include <iostream>

class Watermarker
{
public:
    void mark(std::string& targetDirectory, std::string& watermarkPath);
};

Watermarker.cpp

#include "Watermarker.h"
#include "FileSystem.h"
#include <boost/filesystem.hpp>
#include <opencv2/core.hpp>
#include <opencv2/highgui.hpp>
#include <opencv2/imgproc.hpp>

void Watermarker::mark(std::string& targetDirectory, std::string& watermarkPath)
{
    boost::filesystem::path path;

    // Loop through all the files in the target directory
    for (auto& entry : boost::filesystem::directory_iterator(targetDirectory))
    {
        // Set the boost filesystem path to the path of the file
        path = entry.path();

        FileSystem fileSystem;
        
        // If the file is not valid, skip it
        if (!fileSystem.checkFile(path)) { continue; }

        // Get the original and watermark image
        cv::Mat image = cv::imread(path.string());
        cv::Mat watermarkImage = cv::imread(watermarkPath, cv::IMREAD_UNCHANGED);

        // Make the watermark image the same size as the original image
        cv::resize(watermarkImage, watermarkImage, image.size(), 0, 0, cv::InterpolationFlags::INTER_LINEAR);

        // Give the images 4 channels
        cv::cvtColor(image, image, cv::COLOR_RGB2RGBA);
        cv::cvtColor(watermarkImage, watermarkImage, cv::COLOR_RGB2RGBA);

        cv::Mat newImage;

        // Add the watermark to the original image
        cv::addWeighted(watermarkImage, 0.3, image, 1, 0.0, newImage);

        // Save the new image to the new watermarked_images folder
        cv::imwrite(std::string(targetDirectory + "\watermarked_images\" + path.filename().string()).c_str(), newImage);
    }
}

Question 2

#pragma once не является стандартным C ++ — используйте обычный #ifndef/#define включать охранников до тех пор, пока это не будет стандартизировано (к тому времени модули будут лучшим подходом).

Это выглядит странно:

#include <iostream>

class Watermarker
{
public:
    void mark(std::string& targetDirectory, std::string& watermarkPath);
};

Зачем включать <iostream>? Мы оттуда ничего не используем. Что еще более важно, нам не хватает включения <string>.

Почему mark() нестатическая функция-член? Он не требует ни состояния объекта, ни даже состояния класса. Это может быть бесплатная функция.

Почему мы передаем изменяемые ссылки на пути? Я не думаю, что есть веская причина, по которой вызывающий абонент хотел бы, чтобы какая-либо из этих строк была изменена, поэтому просто возьмите копию (или используйте константную ссылку, если реализация не будет изменять значения).

class FileSystem

Это запутанное имя, учитывая, что мы используем boost::filesystem в том же коде.

int main(int arc, const char** argv)
{
    // Get the given argument for the target directory
    std::string targetDirectory = argv[1];

    // Get the given argument for the watermark file
    std::string watermarkPath = argv[2];

Условно мы называем первый аргумент argcне arc. Более важно то, что мы не можем получить доступ argv[1] и argv[2] если мы не знаем argc составляет не менее 3. А если не точно 3, мы должны сообщить пользователю (через std::cerr), что использование неверно, и выход с ненулевым статусом (например, EXIT_FAILURE, от <cstdlib>).

    if (!fileSystem.checkFile(path)) { continue; }

    // Get the original and watermark image
    cv::Mat image = cv::imread(path.string());

Между проверкой и чтением есть промежуток, когда в многопроцессорной среде может произойти что угодно. И учитывая, что мы никогда не проверяли, есть ли у нас разрешение на чтение файла, cv::imread() может выйти из строя, даже если за это время ничего не изменится. Читать прекрасное руководство:

Если изображение не может быть прочитано (из-за отсутствия файла, неправильных разрешений, неподдерживаемого или недопустимого формата), функция возвращает пустую матрицу (Mat::data==NULL).

Наверное лучше просто удалить FileSystem::checkFile() и полагаться на OpenCV, чтобы сообщить, доступен ли каждый файл для чтения.

    cv::imwrite(std::string(targetDirectory + "\watermarked_images\" + path.filename().string()).c_str(), newImage);

Вот еще одно место, где мы забыли проверить, удалась ли операция. (Честно говоря, пример в руководство тоже делает эту ошибку). Плохая практика — продолжать молча, когда мы не достигли того, о чем просил пользователь.

Также здесь у нас есть фрагмент строки "\watermarked_images\" это должно быть согласовано с кодом вызова. Во-первых, это выглядит так предполагается, что это разделитель путей, который излишне непереносим (особенно с учетом того, что мы используем boost::filesystem). Во-вторых, необходимо передать путь назначения (и было бы лучше, если бы его можно было указать как параметр программы).

Question 3

#pragma once:

Если вы создаете библиотеку, которая должна работать с любым компилятором на любой платформе, вам нужны соответствующие средства защиты. Для чего-то вроде этого #pragma once это путь. (Это проще и менее подвержено ошибкам и поддерживается всеми основными современными компиляторами).

[Other than this I mainly agree with Toby Speight’s answer ;).]

#include порядок:

#include <iostream>
#include "FileSystem.h"
#include "Watermarker.h"

Обычный порядок включения — это размещение локальных включений вверху, сторонняя библиотека включает следующие, а стандартная библиотека включает последние:

#include "FileSystem.h"
#include "Watermarker.h"

#include <boost/filesystem.hpp> // added for illustration

#include <iostream>

Наши локальные заголовки должны #include все, что они используют сами. При таком порядке мы получим ошибку компилятора, если забыли #include что-то в местной шапке.

например, если FileSystem.h зависит от <iostream>, но забывает включить его, мы хотим всегда получаем ошибку компилятора, когда пытаемся использовать FileSystem.h в .cpp файл (чтобы мы могли исправить проблему). Если мы включим <iostream> первый в .cpp файла, мы не получим здесь ошибку компилятора, но может появиться в произвольный момент в будущем, когда мы не будем включать <iostream> тоже.

class Filesystem:

std::vector<std::string> validExtensions должно быть static, поскольку это не относится к экземпляру класса. Фактически, у этого класса нет состояния экземпляра (переменных-членов), поэтому он не обязательно должен быть class. Возможно, мы могли бы сделать это namespace вместо.

Я бы, вероятно, просто поместил весь код этого приложения в одно пространство имен, вместо того, чтобы иметь отдельное пространство имен для нескольких утилит.

Возможно, мы могли бы встроить содержимое этих функций в функции, которые их вызывают, без потери удобочитаемости.

class Watermarker:

Опять же, у этого класса нет членов, поэтому он не обязательно должен быть class.

Хотя верно, что нет смысла проверять, существует ли файл или доступен для чтения, прежде чем мы действительно попытаемся открыть файл, мы все же должны проверить, что наш итератор указывает на файл, а не на каталог, и что он имеет одно из расширений, которые мы заботиться о.

[Another slight disagreement with Toby Speight’s answer.]

Так checkFile() является необходимым. Но я считаю, что его следует разделить на две части, например: isFile(path) и hasExtension(path, validExtensions);.

Я думаю, мы могли бы разделить mark функция на две части. Одна функция (processDir()), чтобы иметь дело с итерацией каталога (здесь мы могли бы просто встроить проверку файла и расширения). Вторая функция addWatermark() для нанесения водяных знаков с помощью OpenCV.

возможное улучшение производительности:

Мы читаем один и тот же файл изображения водяного знака снова и снова.

Мы можем прочитать изображение и преобразовать его в RGBA только один раз. Затем мы можем каждый раз копировать и изменять его размер, что, вероятно, быстрее, чем чтение с диска.

структура проекта:

Хотя разделение частей проекта на разные файлы, как правило, очень хорошо, на самом деле это может сделать вещи менее удобочитаемыми для такого очень небольшого приложения, как это!

validExtensions, например, зависит от приложения и довольно важна, но ее трудно найти в отдельном файле. Между тем Watermarker.cpp также зависит от FileSystem.h (и эти допустимые расширения), что делает его менее читаемым (потому что нам приходится обходить кодовую базу, чтобы понять его) и менее пригодным для повторного использования.

Возможно, в этом случае лучше было бы поместить все в main.cpp, примерно так:

#include <boost/algorithm/string.hpp>
#include <boost/filesystem.hpp>

#include <opencv2/core.hpp>
#include <opencv2/highgui.hpp>
#include <opencv2/imgproc.hpp>

#include <algorithm>
#include <cstdlib>
#include <initializer_list>
#include <iostream>
#include <string>

namespace fs = boost::filesystem;

namespace wtr
{

    void addWatermark(std::string const& targetFile, std::string const& outputFile, std::string const& watermarkFile)
    {
        cv::Mat original = cv::imread(targetFile); // todo: handle failure!
        cv::cvtColor(original, original, cv::COLOR_RGB2RGBA);

        cv::Mat watermark = cv::imread(watermarkFile, cv::IMREAD_UNCHANGED); // todo: handle failure!
        cv::resize(watermark, watermark, image.size(), 0, 0, cv::InterpolationFlags::INTER_LINEAR);
        cv::cvtColor(watermark, watermark, cv::COLOR_RGB2RGBA);

        cv::Mat composite;
        cv::addWeighted(watermark, 0.3, original, 1, 0.0, composite); // todo: make 0.3 a function argument!

        cv::imwrite(outputFile, composite); // todo: handle failure!
    }

    template<class It, class T>
    bool contains(It begin, It end, T const& value)
    {
        return std::find(begin, end, value) != end;
    }

    void processDir(std::string const& targetDir, std::string const& outputDir, std::string const& watermarkFile)
    {
        for (auto& entry : boost::filesystem::directory_iterator(targetDir))
        {
            fs::path path = entry.path();

            if (!fs::is_regular_file(path))
                continue;

            auto validExtensions = { ".png", ".jpg", ".jpeg", ".tiff" };
            if (!contains(validExtensions.begin(), validExtensions.end(), boost::to_lower_copy(path.extension().string())))
                continue;
            
            addWatermark(path.string(), outputDir + path.filename().string(), watermarkFile);
        }
    }

} // wtr

int main(int argc, const char** argv)
{
    // todo: check argc is correct!!!
    std::string targetDir = argv[1];
    std::string watermarkFile = argv[2];

    if (!fs::is_directory(targetDir)) // note: only one check needed - if the file doesn't exist, it won't be a directory!
    {
        std::cout << "The given path is not a directory.n";
        return EXIT_FAILURE;
    }

    std::string outputDir = targetDir + "\watermarked_images";
    fs::create_directory(outputDir);
    // todo: handle failure!!!

    wtr::processDir(targetDir, outputDir, watermarkFile); // note: pass in output directory path instead of recalculating it inside the function.
}

[note: not tested]

Если наша кодовая база растет, или мы хотим повторно использовать функции или переместить их в библиотеку, легко разделить части. Например, сейчас addWatermark зависит только от стандартной библиотеки и OpenCV (поэтому его можно сгруппировать с другими служебными функциями OpenCV), и contains аналогичным образом можно переместить в отдельный файл.

Но … до тех пор, Т.е..

Упрощение применения небольших водяных знаков

2 ответа
2

Добавить комментарий Отменить ответ

2 ответа 2

Похожие записи:

Добавить комментарий Отменить ответ

2 ответа
2