Isso é demais para uma biblioteca apenas para um cabeçalho?

Question

Parece que eu tive que embalar um pouco de código aqui. Estou me perguntando se é uma prática ruim de design deixar isso inteiramente em um arquivo de cabeçalho como este:

#include <list>
#include <string>
#include <boost/noncopyable.hpp>
#include <boost/make_shared.hpp>
#include <boost/iterator/iterator_facade.hpp>
#include <Windows.h>
#include "../Exception.hpp"

namespace WindowsAPI { namespace FileSystem {

class NonRecursiveEnumeration;
class RecursiveEnumeration;
struct AllResults;
struct FilesOnly;

template <typename Filter_T = AllResults, typename Recurse_T = NonRecursiveEnumeration>
class DirectoryIterator;

template <typename Recurse_T>
struct FileData;

class NonRecursiveEnumeration : public boost::noncopyable
{
    WIN32_FIND_DATAW currentData;
    HANDLE hFind;
    std::wstring root;
public:
    NonRecursiveEnumeration() : hFind(INVALID_HANDLE_VALUE) {
    };
    NonRecursiveEnumeration(const std::wstring& pathSpec) {
        std::wstring::const_iterator lastSlash =
            std::find(pathSpec.rbegin(), pathSpec.rend(), L'\\').base();
        if (lastSlash != pathSpec.end())
            root.assign(pathSpec.begin(), lastSlash);
        hFind = FindFirstFileW(pathSpec.c_str(), &currentData);
        if (hFind == INVALID_HANDLE_VALUE)
            WindowsApiException::ThrowFromLastError();
        while (!wcscmp(currentData.cFileName, L".") || !wcscmp(currentData.cFileName, L"..")) {
            increment();
        }
    };
    void increment() {
        BOOL success =
            FindNextFile(hFind, &currentData);
        if (success)
            return;
        DWORD error = GetLastError();
        if (error == ERROR_NO_MORE_FILES) {
            FindClose(hFind);
            hFind = INVALID_HANDLE_VALUE;
        } else {
            WindowsApiException::Throw(error);
        }
    };
    ~NonRecursiveEnumeration() {
        if (hFind != INVALID_HANDLE_VALUE)
            FindClose(hFind);
    };
    bool equal(const NonRecursiveEnumeration& other) const {
        if (this == &other)
            return true;
        return hFind == other.hFind;
    };
    const std::wstring& GetPathRoot() const {
        return root;
    };
    const WIN32_FIND_DATAW& GetCurrentFindData() const {
        return currentData;
    };
};

//Not implemented yet
class RecursiveEnumeration : public boost::noncopyable
{
};

template <typename Recurse_T>
struct FileData //Serves as a proxy to the WIN32_FIND_DATA struture inside the iterator.
{
    const Recurse_T* impl;
    template <typename Filter_T, typename Recurse_T>
    FileData(const DirectoryIterator<Filter_T, Recurse_T>* parent) : impl(parent->impl.get()) {};
    DWORD GetAttributes() const {
        return impl->GetCurrentFindData().dwFileAttributes;
    };
    bool IsDirectory() const {
        return (GetAttributes() & FILE_ATTRIBUTE_DIRECTORY) != 0;
    };
    bool IsFile() const {
        return !IsDirectory();
    };
    bool IsArchive() const {
        return (GetAttributes() & FILE_ATTRIBUTE_ARCHIVE) != 0;
    };
    bool IsReadOnly() const {
        return (GetAttributes() & FILE_ATTRIBUTE_READONLY) != 0;
    };
    unsigned __int64 GetSize() const {
        ULARGE_INTEGER intValue;
        intValue.LowPart = impl.GetCurrentFindData().nFileSizeLow;
        intValue.HighPart = impl.GetCurrentFindData().nFileSizeHigh;
        return intValue.QuadPart;
    };
    std::wstring GetFolderPath() const {
        return impl->GetPathRoot();
    };
    std::wstring GetFileName() const {
        return impl->GetCurrentFindData().cFileName;
    };
    std::wstring GetFullFileName() const {
        return GetFolderPath() + GetFileName();
    };
    std::wstring GetShortFileName() const {
        return impl->GetCurrentFindData().cAlternateFileName;
    };
    FILETIME GetCreationTime() const {
        return impl->GetCurrentFindData().ftCreationTime;
    };
    FILETIME GetLastAccessTime() const {
        return impl->GetCurrentFindData().ftLastAccessTime;
    };
    FILETIME GetLastWriteTime() const {
        return impl->GetCurrentFindData().ftLastWriteTime;
    };
};

struct AllResults
{
    template <typename Recurse_T>
    bool operator()(const FileData<Recurse_T>&) {
        return true;
    };
}; 

struct FilesOnly
{
    template <typename Recurse_T>
    bool operator()(const FileData<Recurse_T>& arg) {
        return arg.IsFile();
    };
};

#pragma warning(push)
#pragma warning(disable: 4355)
template <typename Filter_T, typename Recurse_T>
class DirectoryIterator : public boost::iterator_facade<DirectoryIterator<Filter_T>, const FileData<Recurse_T>, std::input_iterator_tag>
{
    friend class boost::iterator_core_access;
    boost::shared_ptr<Recurse_T> impl;
    FileData<Recurse_T> derefData;
    Filter_T filter;
    void increment() {
        do {
            impl->increment();
        } while (! filter(derefData));
    };
    bool equal(const DirectoryIterator& other) const {
        return impl->equal(*other.impl);
    };
    const FileData<Recurse_T>& dereference() const {
        return derefData;
    };
public:
    typedef FileData<Recurse_T> DataType;
    friend struct DataType;
    DirectoryIterator(Filter_T functor = Filter_T()) :
        impl(boost::make_shared<Recurse_T>()),
        derefData(this),
        filter(functor) {
    };
    explicit DirectoryIterator(const std::wstring& pathSpec, Filter_T functor = Filter_T()) :
        impl(boost::make_shared<Recurse_T>(pathSpec)),
        derefData(this),
        filter(functor) {
    };
};
#pragma warning(pop)

}}

Answer 1

Tenho muito mais código em alguns dos meus, se isso é de algum consolo. E também as implementações da biblioteca padrão C ++, Boost e Microsoft (por exemplo, ATL).

Answer 2

A única parte que me parece aberta a muita pergunta seria implementações das funções em DirectoryIteratorImpl. Não é um modelo, por isso realmente não precisa estar em um cabeçalho e possui algumas rotinas um pouco mais longas (o construtor "real" e o incremento).

O restante são modelos ou então compostos por tais funções triviais que você deseja que eles sejam alinhados em qualquer caso (por exemplo, os membros de FileData). Esses acabarão em um cabeçalho em qualquer caso.

Answer 3

Quanto ao comprimento do cabeçalho, você pode ter tanto código quanto quiser nos arquivos do cabeçalho. A troca é a quantidade de código que deve ser recompilada sempre que seu programa for construído; O código colocado em seus arquivos CPP pode ser compilado em arquivos de objeto e vinculado a cada compilação subsequente.

Eu sugeriria que cada uma das definições de método DirectoryIteratorImpl deve ser movido para um .cpp Arquivo. Se você não está definindo um método em linha dentro de uma definição de classe, não há razão para que ela seja incluída no arquivo de cabeçalho.

Um de lado não relacionado: evite escrever inline DirectoryIteratorImpl(); - Na verdade, escreva suas funções embutidas em linha ou não as marque em linha. De C ++ FAQ Lite:

Geralmente é imperativo que a definição da função (a parte entre o {...}) seja colocada em um arquivo de cabeçalho. Se você colocar a definição da função em linha em um arquivo .cpp e se for chamado de algum outro arquivo .cpp, receberá um erro "não resolvido externo" do vinculador.

Se suas funções forem "grandes demais" para escrever no arquivo de cabeçalho, elas são grandes demais para embelezar e o compilador provavelmente ignorará sua sugestão em linha de qualquer maneira.

Answer 4

Parece que você está programando para o Windows aqui, assumiremos que você está usando o Visual Studio?

De qualquer forma, acho que não há algo como código demais nos cabeçalhos.

É uma questão de comércio principalmente:

• Compilação mais lenta (mas temos multicores e cabeçalhos pré -compilados)
• Recompilação mais frequente (novamente, multicores)
• Talvez o código inchaço ...

O único ponto que é irritante (na minha opinião) é o mais recente ... e vou precisar de ajuda aqui: temos certeza de eles e transformá -los em uma chamada regular?

Francamente, eu não me preocuparia muito com isso. Um número de Boost As bibliotecas são somente para o cabeçalho, mesmo para suas peças que não são de templos, simplesmente porque facilita a integração (sem necessidade de vinculação).