HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Reading binary file of doubles

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readingfilebinarydoubles

Problem

I'm trying to read doubles from a relatively small binary file. This currently reads a 100 KB file in about 6 milliseconds in my system. I would like to reduce that if possible.

void readNParseData(const char filePath, vector &data){

ifstream ifs(filePath, ios::in | ios::binary);

// If this is a valid file
if (ifs) {
// Temporary Variables
std::streampos fileSize;
double *fileBuffer;
size_t sizeOfBuffer;

// Check whether the parameter is already full
if (data != 0){
// Reset the output
data->clear();
data = 0;
}

// Get the size of the file
ifs.seekg(0, std::ios::end);
fileSize = ifs.tellg();
ifs.seekg(0, std::ios::beg);

sizeOfBuffer = fileSize / sizeof(double);
fileBuffer = new double[sizeOfBuffer];

ifs.read(reinterpret_cast(fileBuffer), fileSize);

// Now convert the double array into vector
data = new vector(fileBuffer, fileBuffer + sizeOfBuffer);

free(fileBuffer);
}
}


As you can see there is a redundant copy of a double * array to a vector. Perhaps reading to the vector directly might speed it up, but I don't know how.

Solution

Suggestion 1

This block of code does not seem clean:

// Check whether the parameter is already full
    if (data != 0){
        // Reset the output
        data->clear();
        data = 0;
    }


If data used to point to non-NULL, then you are just making it NULL. You should delete data before pointing it to NULL:

if (data != 0){
        // Reset the output
        data->clear();
        delete data;
        data = 0;
    }


Suggestion 2

Still better, if you have the option, change the interface to

void readNParseData(const char* filePath, vector& data);


Suggestion 3

There is nothing in your code to indicate to the calling function that you were able or unable to read the data from the file. There is no else to go with

if (ifs) {


One way of indicating whether the function was successful in reading the data is to change the return type of the function to bool. Then, you can add

return true;


at the end of the if block and then add an else block:

else {
   return false;
}


Suggestion 4

To remove the redundant memory allocation and deallocation to fileBuffer, simply use std::vector::data:

data.resize(sizeOfBuffer);
ifs.read(reinterpret_cast(data.data()), fileSize);


Suggestion 5

Add a check to make sure that ifs.read was successful:

ifs.read(reinterpret_cast(data.data()), fileSize);
if ( !ifs )
{
   return false;
}

Code Snippets

// Check whether the parameter is already full
    if (data != 0){
        // Reset the output
        data->clear();
        data = 0;
    }
if (data != 0){
        // Reset the output
        data->clear();
        delete data;
        data = 0;
    }
void readNParseData(const char* filePath, vector<double>& data);
return true;
else {
   return false;
}

Context

StackExchange Code Review Q#75147, answer score: 10

Revisions (0)

No revisions yet.