Разделение файла и передача данных другим классам

Question

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

Вот упрощенный пример моего текущего подхода:

class Archive {
    private:
        std::istream &fs;
        void Read();
    public:
        Archive(std::istream &fs); // Calls Read() automatically
        ~Archive();
        const char* Get(int archiveIndex);
        size_t GetSize(int archiveIndex);
};

class FileFormat {
    private:
        std::istream &fs;
        void Read();
    public:
        FileFormat(std::istream &fs); // Calls Read() automatically
        ~FileFormat();
};

Класс Archive в основном анализирует архив и считывает сохраненные файлы в char указатели. Чтобы загрузить первый FileFormat файл из Archive, я бы в настоящее время использовал следующий код:

* * 1010

(Обратите внимание, что некоторые файлы в архиве могут быть дополнительными архивами, но другого формата.)

При чтении двоичных данных я использую класс BinaryReader с такими функциями:

BinaryReader::BinaryReader(std::istream &fs) : fs(fs) {
}

char* BinaryReader::ReadBytes(unsigned int n) {
    char* buffer = new char[n];
    fs.read(buffer, n);
    return buffer;
}

unsigned int BinaryReader::ReadUInt32() {
    unsigned int buffer;
    fs.read((char*)&buffer, sizeof(unsigned int));
    return buffer;
}

Мне нравится простота этого подхода, но в настоящее время я борюсь с большим количеством ошибок памяти и SIGSEGV, и я боюсь, что это из-за этого метода. Пример - когда я многократно создаю и читаю архив в цикле. Он работает большое количество итераций, но через некоторое время вместо этого начинает читать ненужные данные.

У меня к вам вопрос: возможен ли такой подход (в таком случае я спрашиваю, что я делаю неправильно), а если нет, какие существуют лучшие подходы?

Answer 1

Недостатки кода в ОП:

1. Вы выделяете кучу памяти и возвращаете указатель на нее из одной из ваших функций. Это может привести к утечке памяти. У вас нет проблем с утечками (пока), но вы должны иметь в виду такие вещи при разработке своих классов.
2. При работе с классами Archive и FileFormat пользователь всегда должен учитывать внутреннюю структуру вашего архива. По сути, это ставит под угрозу идею инкапсуляции данных.

Когда пользователь вашей платформы классов создает объект Archive, он просто получает способ извлечь указатель на некоторые необработанные данные. Затем пользователь должен передать эти необработанные данные полностью независимому классу. Также у вас будет более одного вида FileFormat. Даже без необходимости следить за распределением утечки в куче, имеющей отношение к такой системе, она будет подвержена ошибкам.

Давайте попробуем применить некоторые принципы ООП к задаче. Ваш объект Archive является контейнером файлов различного формата. Таким образом, эквивалент Get () в архиве обычно должен возвращать объекты File, а не указатель на необработанные данные:

//We gonna need a way to store file type in your archive index
enum TFileType { BYTE_FILE, UINT32_FILE, /*...*/ }

class BaseFile {
public:
virtual TFileType GetFileType() const = 0;
/* Your abstract interface here */
};

class ByteFile : public BaseFile {
public:
ByteFile(istream &fs);
virtual ~ByteFile();
virtual TFileType GetFileType() const
{ return BYTE_FILE; }
unsigned char GetByte(size_t index);
protected:
/* implementation of data storage and reading procedures */
};

class UInt32File : public BaseFile {
public:
UInt32File(istream &fs);
virtual ~UInt32File();
virtual TFileType GetFileType() const
{ return UINT32_FILE; }
uint32_t GetUInt32(size_t index);
protected:
/* implementation of data storage and reading procedures */
};


class Archive {
public:
Archive(const char* filename);
~Archive();
BaseFile* Get(int archiveIndex);
{ return (m_Files.at(archiveIndex)); }
/* ... */
protected:
vector<BaseFile*> m_Files;
}

Archive::Archive(const char* filename)
{
    ifstream fs(filename);

    //Here we need to:
    //1. Read archive index
    //2. For each file in index do something like:
    switch(CurrentFileType) {
    case BYTE_FILE:
           m_Files.push_back(new ByteFile(fs));
           break;
    case UINT32_FILE:
           m_Files.push_back(new UInt32File(fs));
           break;
    //.....
    }
}  

Archive::~Archive()
{
    for(size_t i = 0; i < m_Files.size(); ++i)
        delete m_Files[i];
}

int main(int argc, char** argv)
{
     Archive arch("somearchive.arc");
     BaseFile* pbf;
     ByteFile* pByteFile;

     pbf = arch.Get(0);

     //Here we can use GetFileType() or typeid to make a proper cast
     //An example of former:

     switch ( pbf.GetFileType() ) {
     case BYTE_FILE:
         pByteFile = dynamic_cast<ByteFile*>(pbf);
         ASSERT(pByteFile != 0 );
         //Working with byte data
         break;
     /*...*/
     }

     //alternatively you may omit GetFileType() and rely solely on C++ 
     //typeid-related stuff

}

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

Имейте в виду, что хороший дизайн класса может помочь вам в предотвращении утечек памяти, разъяснения кода и тому подобного. Но какие бы у вас ни были классы, вы все равно будете иметь дело с проблемами хранения двоичных данных. Например, если в вашем архиве хранятся 64 байта байтовых данных и 8 uint32, и вы как-то читаете 65 байтов вместо 64, чтение следующих целых даст вам мусор. Также могут возникнуть проблемы с выравниванием и порядком байтов (последнее важно, если ваши приложения должны работать на нескольких платформах). Тем не менее, хороший дизайн класса может помочь вам создать лучший код, который решает такие проблемы.

Answer 2

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

Так что

Foo * createFoo();

, скорее всего, будет функцией, которая создает объект, который пользователь должен удалить.

Предпочтительным решением для начала было бы вернуть std::vector<char> или разрешите пользователю передавать std::vector<char> & в вашу функцию, и вы записываете в нее байты, устанавливая ее размер при необходимости.(Это более эффективно, если выполнять несколько операций чтения, когда вы можете повторно использовать один и тот же буфер).

Вы также должны изучить правильность const.

Что касается вашего "через некоторое время он заполняется мусором"где вы проверяете конец файла?