Why saving to a file is not working as expected?

Question

I have this struct:

struct Employee
{
   char VarOne[50];        
   unsigned int VarTwo;      
   double VarThree[4];           
}

Then, I populate a dynamic array of this struct:

 Employee* MyArray = new Employee[TheSize]; // Sorry I forgot to mention TheSize is = 5 constant

Then, I try and write the array in binary mode to a file:

   // write as binary
   fstream OutFileBin;
   OutFileBin.open("Employee.dat", ios::binary | ios::out);
   OutFileBin.write(reinterpret_cast<char *>(&MyArray), TheSize * sizeof(Employee));
   OutFileBin.close();

But when I read the file in binary mode, it fails and data are junk:

   // read as binary
   fstream InFilebin;
   InFilebin.open("Employee.dat", ios::binary | ios::in);
   Employee NewArray[TheSize]; // sorry I forgot to mention TheSize is = 5 constant
   InFilebin.read(reinterpret_cast<char *>(&NewArray), TheSize * sizeof(Employee));

What is that I am doing wrong?

I do not know why the down and the close vote, I showed my work in a [MCVE] and I am expecting results from the .dat file to not be junk ... — Sandra K, Mar 20 '19 at 17:53
Why are you not using std::vector? What is `TheSize`? And it's not complete - it won't compile. — , Mar 20 '19 at 17:54
Well, I think that's a typo `&MyArray` . Shouldn't that be just `MyArray` for writing and reading? Also I highly recommend your `reinterpret_cast` should rather be `reinterpret_cast` to save you from unexpected behaviors. — πάντα ῥεῖ, Mar 20 '19 at 17:54
If you have an MCVE please show it, I cannot see one in your current question. Maybe read the link again and double check that the shown code matches all the attributes described there. Providing input data is considered part of an MCVE. — Yunnosch, Mar 20 '19 at 17:55
@NeilButterworth I am not allowed to use std::vector. And TheSize can be any number, say it is a 5.. — Sandra K, Mar 20 '19 at 17:55
I find myself wondering why dynamic allocation here: `Employee* MyArray = new Employee[TheSize];`, but not here: `Employee NewArray[TheSize];`. If `TheSize` is constant, both should be Automatic variables to keep the memory management as simple as possible unless `TheSize` is too large to fit the array in Automatic storage, which sentences case 2 to an ill doom. If `TheSize` is not constant, case 2 requires a non--Standard compiler extension and will not compile on many compilers. — user4581301, Mar 20 '19 at 18:02
I am sorry I forgot to mention `TheSize` is = 5 constant, so I did not notice while testing/posting. I edited my Q — Sandra K, Mar 20 '19 at 18:06
This doesn't address the question, but get in the habit of initializing objects with meaningful values rather than creating them with default values and immediately changing them. That is, change `fstream OutFileBin; OutFileBin.open("Employee.dat", ios::binary | ios::out);` to `fstream OutFileBin("Employee.dat", ios::binary | ios::out);`. Also, you don't need to call `OutFileBin.close();`. The destructor will do that. — Pete Becker, Mar 20 '19 at 18:07

R Sahu · Accepted Answer · 2019-03-20T17:58:52.387

3

The line

OutFileBin.write(reinterpret_cast<char *>(&MyArray), TheSize * sizeof(Employee));

is not good. You don't want to treat &MyArray as though it stores objects of type Employee. It needs to be just MyArray.

OutFileBin.write(reinterpret_cast<char*>(MyArray), TheSize * sizeof(Employee));

Also,

Employee NewArray[TheSize];

is not standard C++ unless TheSize is a compile time constant. Change it to

Employee* NewArray = new Employee[TheSize];

and the following line to

InFilebin.read(reinterpret_cast<char *>(NewArray), TheSize * sizeof(Employee));

edited Mar 20 '19 at 17:58

answered Mar 20 '19 at 17:55

R Sahu

204,454
14
159
270

Your solution fixed it! But it is is not what this gentleman did!, why? https://stackoverflow.com/a/5506657/6540193 – Sandra K Mar 20 '19 at 17:58
See the additions regarding `uint8_t` in [my comment](https://stackoverflow.com/questions/55267221/why-saving-to-a-file-is-not-working-as-expected#comment97265766_55267221) please. – πάντα ῥεῖ Mar 20 '19 at 17:58
1

@SandraK, The numerical value of `&array` is the same as `array` iff `array` is a statically defined array. That's why the answer in the linked post works. Notice how they are able to use `sizeof(apprentice)`. That wont work for dynamic arrays either. – R Sahu Mar 20 '19 at 18:01
@SandraK You might also want to know about [What is array decaying](https://stackoverflow.com/questions/1461432/what-is-array-decaying). – πάντα ῥεῖ Mar 20 '19 at 18:03
@πάνταῥεῖ, you'll have to educate me on why use of `reinterpret_cast` is not as safe as `reinterpret_cast`. The standard library expects a `char const*` for `ostream::write` and a `char*` for `istream::read`. – R Sahu Mar 20 '19 at 18:04
@RSahu My point is that `uint8_t` represents simple bytes, rather implicit (implementation dependent) of `signed char` or `unsigned char`. It could make at least a difference if the binary file is shared among different machine architectures or compilers used. Correct me if I am wrong about this regarding the standard definitions please. – πάντα ῥεῖ Mar 20 '19 at 18:06

Why saving to a file is not working as expected?

1 Answers1