patterncppModerate
Reading binary file of doubles
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.
As you can see there is a redundant copy of a
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:
If
Suggestion 2
Still better, if you have the option, change the interface to
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
One way of indicating whether the function was successful in reading the data is to change the return type of the function to
at the end of the
Suggestion 4
To remove the redundant memory allocation and deallocation to
Suggestion 5
Add a check to make sure that
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 withif (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 addreturn 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.