0

I have struct

struct Order
{
    unsigned int productamount = 0;
    Products product = Products::OOPlabs;
    double cost = 0.0;
    string FIO = "Иванов Иван Иванович";
    unsigned int orderID = 0;
};

and functions to binary write and read array of this structs

bool createbinfile(string way, Order* request, int reqlen)
{
    ofstream f(way, ios::trunc | ios::binary);
    if (!f.is_open())
    {
        cout << "Файл не найден\n";
        return false;
    }
    else if (f.rdstate())
    {
        cout << "Ошибка неизвестной природы\n";
        return false;
    }
    f.write((char*)&reqlen, sizeof(int));
    for (int i = 0; i < reqlen; i++)
    {
        f.write(reinterpret_cast<char *>(&request[i].productamount), sizeof(unsigned int));
        f.write(reinterpret_cast<char *>(&request[i].product), sizeof(Products));
        f.write(reinterpret_cast<char *>(&request[i].cost), sizeof(double));
        size_t tmp = request[i].FIO.length();
        f.write(reinterpret_cast<char *>(&tmp), sizeof(size_t));
        f.write(&request[i].FIO[0], tmp);
    }
    f.close();
    return true;
}

bool readbinfile(string way, Order* &request, int &len)
{
    ifstream f(way, ios::binary);
    if (!f.is_open())
    {
        cout << "Файл не найден\n";
        return false;
    }
    else if (f.rdstate())
    {
        cout << "Ошибка неизвестной природы\n";
        return false;
    }
    f.read(reinterpret_cast<char *>(&len), sizeof(int));
    for (int i = 0; i < len; i++)
    {
        f.read(reinterpret_cast<char *>(&request[i].productamount), sizeof(unsigned int));
        f.read(reinterpret_cast<char *>(&request[i].product), sizeof(Products));
        f.read(reinterpret_cast<char *>(&request[i].cost), sizeof(double));
        size_t tmp = 0;
        f.read(reinterpret_cast<char *>(&tmp), sizeof(size_t));
        request[i].FIO.resize(tmp);
        f.read(&request[i].FIO[0], tmp);
    }
    f.close();
    return true;
}

There is some problem with string. I can read and write it in binary mode during the execution, but after restarting the program i can't read it - having "Access violation writing location". Why? It's repeated if I write it character - by - character. How can I fix it? The problem is about null-terminating, serialization, type conversion, or what? I'm confused. Did I miss something?

Kir S
  • 15
  • 1
  • 3
  • 10
  • 1
    `f.write(reinterpret_cast(...))`.... but.... **why**? This isn't C, and even in C you would probably go `printf()` instead of dumping raw binary... – DevSolar Dec 04 '18 at 19:02
  • A `std::string` is not a POD type. You *cannot* serialize it by simply writing the raw bytes of the object (hint: it contains pointer members). – Jesper Juhl Dec 04 '18 at 19:06
  • Before reading a string, you have to resize its buffer like this: `request[i].FIO.resize(tmp)`. Also it should be `f.read(&request[i].FIO[0], tmp);` because `tmp` contains the string size. `sizeof(tmp)` gives you only the size of the variable `tmp`, not the string size stored **in** tmp. – zett42 Dec 04 '18 at 19:07
  • Another error: `f.write(&request[i].FIO[0], sizeof(tmp));` --> should be `f.write(&request[i].FIO[0], tmp);` – zett42 Dec 04 '18 at 19:12
  • @Jesper OP tries to write size of string, then the character array, not just the "raw bytes" of the object. The mistakes are in the implementation, not in the concept, as outlined by my previous comments. – zett42 Dec 04 '18 at 19:16
  • @KirS: Check out any C++ I/O tutorial, and the niceness that is `operator<<`... – DevSolar Dec 04 '18 at 19:16
  • @DevSolar I thought that operator doesn't work with binary mode – Kir S Dec 04 '18 at 19:20
  • @zett42 Whoops. My bad. – Jesper Juhl Dec 04 '18 at 19:21
  • @zett42 So can I do something more? – Kir S Dec 04 '18 at 19:50
  • @KirS: Not *using* binary mode and not writing raw binary is kind of the point... you *can* write binary files for serialization, but you shouldn't do it the way you showcased here. – DevSolar Dec 04 '18 at 20:05
  • @Devsolar So what way should I do it? Where can I search? – Kir S Dec 04 '18 at 20:12
  • "Check out any C++ I/O tutorial..." -- Or a good [book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – DevSolar Dec 04 '18 at 20:16

1 Answers1

1

It's usually good to add streaming operators to the class/struct since it breaks down a big problem into smaller parts. I've made an example here. I've added a class (OrderBox) for keeping many Order objects and added streaming operators for that class too. I don't write the length of the FIO field to the file but instead read/write until \0. Since \0 is perfectly valid in the middle of a string (but I assume that you will not use strings like that), I make sure that \0 in the middle of a string is never saved to file (look for the std::strlen in Orders ofstream operator). Should work in C++14/17/2a.

#include <iostream>
#include <initializer_list>
#include <fstream>
#include <vector>
#include <cstring>
#include <stdexcept>

enum Products {OOPlabs}; // no idea if this is an enum, needed it to compile ...

//-----------------------------------------------------------------------------
// slightly restructured - I put the FIO field last
struct Order
{
    unsigned int productamount = 0;
    Products product = Products::OOPlabs;
    double cost = 0.0;
    unsigned int orderID = 0;
    std::string FIO = "Иванов Иван Иванович";

    // operator to read from file
    friend std::ifstream& operator>>(std::ifstream&, Order&);

    // operator to write to file
    friend std::ofstream& operator<<(std::ofstream&, const Order&);

    // operator to stream to other ostreams, like std::cout
    friend std::ostream& operator<<(std::ostream&, const Order&);
};

using OrderVec = std::vector<Order>;

//-----------------------------------------------------------------------------
// read one Order from a file stream
std::ifstream& operator>>(std::ifstream& is, Order& ord) {
    is.read(reinterpret_cast<char*>(&ord.productamount), sizeof(ord.productamount));
    is.read(reinterpret_cast<char*>(&ord.product), sizeof(ord.product));
    is.read(reinterpret_cast<char*>(&ord.cost), sizeof(ord.cost));
    is.read(reinterpret_cast<char*>(&ord.orderID), sizeof(ord.orderID));
    std::getline(is, ord.FIO, '\0');
    return is;
}

// write one Order to a file stream
std::ofstream& operator<<(std::ofstream& os, const Order& ord) {
    os.write(reinterpret_cast<const char*>(&ord.productamount), sizeof(ord.productamount));
    os.write(reinterpret_cast<const char*>(&ord.product), sizeof(ord.product));
    os.write(reinterpret_cast<const char*>(&ord.cost), sizeof(ord.cost));
    os.write(reinterpret_cast<const char*>(&ord.orderID), sizeof(ord.orderID));
    // using strlen in case a '\0' has snuck into the string
    os.write(ord.FIO.c_str(), std::strlen(ord.FIO.c_str())+1);
    return os;
}

// stream an Order ... to std::cout
std::ostream& operator<<(std::ostream& os, const Order& ord) {
    os << "{" << ord.productamount << "," << ord.product << "," << ord.cost
       << "," << ord.orderID << "," << ord.FIO << "}";
    return os;
}
//-----------------------------------------------------------------------------
class OrderBox { // to keep all your Order objects
    OrderVec m_orders;
public:
    // default ctor
    OrderBox() : m_orders() {}

    // ctor to read from a file
    OrderBox(const std::string file) :
        m_orders()
    {
        if(!readbinfile(file))
            throw std::runtime_error("error reading file");
    }

    // ctor to populate from an initializer_list {...}
    OrderBox(std::initializer_list<Order> il) :
        m_orders(il)
    {}

    bool createbinfile(const std::string& filename)
    {
        std::ofstream f(filename, std::ios::trunc | std::ios::binary);
        if(f) f << *this; // use OrderBox's ofstream operator
        return f.good();
    }

    bool readbinfile(const std::string& filename)
    {
        std::ifstream f(filename, std::ios::binary);
        if(f) f >> *this; // use OrderBox's ifstream operator
        return f.good();
    }

    // the OrderBox's stream operators
    friend std::ifstream& operator>>(std::ifstream&, OrderBox&);
    friend std::ofstream& operator<<(std::ofstream&, const OrderBox&);
    friend std::ostream& operator<<(std::ostream&, const OrderBox&);
};
//-----------------------------------------------------------------------------
std::ifstream& operator>>(std::ifstream& is, OrderBox& ob) {
    OrderVec result;
    Order tmpord;
    size_t reqlen;

    is.read(reinterpret_cast<char*>(&reqlen), sizeof(reqlen));
    result.reserve(reqlen);

    while(is>>tmpord) // use the ifstream operator of Order
    {
        result.emplace_back(std::move(tmpord));
        if(result.size()==reqlen) break; // all records read
    }
    if(result.size()!=reqlen) is.setstate(std::ios_base::failbit);
    else std::swap(result, ob.m_orders);
    return is;
}

std::ofstream& operator<<(std::ofstream& os, const OrderBox& ob) {
    size_t reqlen = ob.m_orders.size();
    os.write(reinterpret_cast<const char*>(&reqlen), sizeof(reqlen));
    for(const auto& ord : ob.m_orders)
    {
        os << ord; // use the ofstream operator of Order
    }
    return os;
}

std::ostream& operator<<(std::ostream& os, const OrderBox& ob) {
    os << "{\n";
    for(const auto& ord : ob.m_orders) {
        std::cout << " " << ord << "\n"; // print one order
    }
    os << "}\n";
    return os;
}
//-----------------------------------------------------------------------------
int main() {
    OrderBox ob = { // OrderBox with orders to write to file
        {10, OOPlabs, 0.1, 1, "Hello One"},
        {20, OOPlabs, 0.2, 2, "Hello Two"},
        {30, OOPlabs, 0.3, 3, "Hello Three"},
        {40, OOPlabs, 0.4, 4, "Hello Four"},
        {50, OOPlabs, 0.5, 5, "Hello Five"}
    };

    try {
        if(ob.createbinfile("orders.db")) {
            // a new OrderBox to populate directly from the file
            OrderBox newbox("orders.db");
            // stream OrderBox
            std::cout << newbox;
        }
    } catch(const std::exception& ex) {
        std::clog << "Exception: " << ex.what() << "\n";
    }
}

Output if it all works:

{
 {10,0,0.1,1,Hello One}
 {20,0,0.2,2,Hello Two}
 {30,0,0.3,3,Hello Three}
 {40,0,0.4,4,Hello Four}
 {50,0,0.5,5,Hello Five}
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • I have c++17 but in `is.read(ord.FIO.data(), len);` ord.FIO.data() gives error: `const char* is incompatible with parameter type of char*` – Kir S Dec 04 '18 at 21:13
  • Are you sure you are compiling in C++17 mode? That's exactly what was added to C++17. Which compiler are you using? I could made a mod to not write the string length to file but to use `\0` as terminator instead. That works as long as you don't need strings with `\0` in the middle of them. – Ted Lyngmo Dec 04 '18 at 21:19
  • Thanks for your concern! Maybe your answer will be helpful for someone. – Kir S Dec 04 '18 at 21:54
  • Sure, but I'll put up a new version that doesn't require C++17. A few minutes. – Ted Lyngmo Dec 04 '18 at 22:30
  • I've now made it C++14 friendly. :-) – Ted Lyngmo Dec 04 '18 at 22:46
  • So at the end @AlexGlebe found that to solve my problem after reading size of array of structs I need to allocate memory for the whole array: `request = (Order*)operator new (sizeof(Order)*len);` and call constructors of structs to fill them with default values: `for(size_t i=len;i>0;){--i; new (request + i) Order;}` so error vanishes – Kir S Dec 05 '18 at 04:24
  • I'm not sure I understand. No such thing should be necessary. There's no need to use `new` anywhere in a program llike this. The `OrderBox` uses a `vector` to fill with the `Order`s. It will expand as needed - but to make it slightly more effective I added a `reserve` to `operator>>(std::ifstream& is, OrderBox& ob)` just now. – Ted Lyngmo Dec 05 '18 at 06:40
  • I didn't mention: I've had a task about array implementation, that need to be expanded, as now I see, but I didn't get it in time. Comment solves that problem. I will try to describe the question in more detail next time. – Kir S Dec 05 '18 at 06:54
  • I see. To save yourself from future problems, keep that part of the assignment completely separate and implement your own type agnostic `kir::array` template. Then replace the line `using OrderVec = std::vector;` above with `using OrderVec = kir::array;` You should try to avoid `new`/`delete` in the actual program you write. – Ted Lyngmo Dec 05 '18 at 09:37