1
\$\begingroup\$

Consider the following contrived data type:

Building: int age, char* location, float quality

The location is in the format "XXX-XXX" (exactly 7 characters).

Goal: Serialize the object into a binary format and then deserialize the object to display (on console) the exact same building info as inputed by the user.

For example, suppose the user imputed the building object: age = 45, location = "345-789", quality = "2.3829.

Then the program should display on the console:

age = 45, location = 345-789, quality = 2.3829

Attempt: Although my application works exactly as specified by the goal, I believe there are major inefficiencies in my code. I am asking for feedback directly related to efficiency. Does there exist a more efficient method to accomplish the same goal? This project is for educational purposes and I cannot use any serialization libraries.

#include <iostream>
#include <sstream>
#include <fstream>
#include <iomanip>

class Building
{
private:
    int m_age; // the age of the building
    char m_locationCode[7]; // location is the format:: example "357-278" (exactly 7 characters)
    float m_buildingQuality; // the quality of the building (scale from 0 - 1000)
    int m_buildingQualityDecimals; // the number of digits after the decimal for building quality
public:
    Building(int age, const std::string& loc, const std::string& quality)
    {
        m_age = age;
        for(unsigned i = 0; i < loc.size(); i++)
            m_locationCode[i] = loc[i];
        char qualityBuffer[10];
        int count = 0;
        bool hitDecimal = false;
        for(int i = 0; i < quality.size(); i++)
        {
            qualityBuffer[i] = quality[i];
            if(quality[i] == '.')
            {
                hitDecimal = true;
            }
            if(hitDecimal == true)
                count++;
        }
        m_buildingQuality = atof(qualityBuffer);
        m_buildingQualityDecimals = count;

    }
    int getAge() {return m_age;}
    std::string getLoc()
    {
        std::string toReturn(m_locationCode, 7);
        return toReturn;

    };
    int getDecimals()
    {
        return m_buildingQualityDecimals;
    }
    float getQuality()
    {
        return m_buildingQuality;
    }
};

int main()
{
    Building b(345, "123-456", "3.23654");

    // Serializing
    std::ofstream file;
    file.open("BinaryBuilding.bin", std::ios::out | std::ios::binary | std::ios::trunc);
    file.write((char*) &b, sizeof(b)); // writing file to binary by casting to a char*
    file.close();

    // Deserialzing
    std::ifstream file2;
    file2.open("BinaryBuilding.bin", std::ios::in | std::ios::binary);
    Building* b2 = new Building(0, "000-000", "0.0");
    file2.read((char *) b2, sizeof(*b2));
    file2.close();

    // Testing process
    std::cout << "Age: " << b2->getAge() << "; Loc: " << b2->getLoc() <<
                "; Building Quality:" <<  std::setprecision(b2->getDecimals()) << b2->getQuality() <<std::endl;

    delete b2;
    return 0;
}

Console Output: Age: 345; Loc: 123-456; Building Quality: 3.23654

Remarks: Notice that I parse the number of characters after the decimal point as inputted by the user. I think this is inefficient since I must store another int inside the class. This is slower and increases the size of the binary file. I want to be able to serialize and deserialize this object as fast as possible. I will eventually extend this to parse millions of objects. My professor told me that the location (i.e. XXX-XXX) should not be written as ASCII. How could the task be accomplished without writing the location to ASCII?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Binary Serialization

Is a bit more complex that what you have implemented.
You have serialized the content of the memory that is occupied by an object of type Building. Which is not quite the same as binary serialization of an object.

Problem 1: Integers are not represented the same on all platforms.
Problem 2: Floats are not represented the same on all platforms.

Those are the obvious two. The not so obvious ones are packing added by the compiler to the object. Changing the compiler or even just the compiler flags can change both the packing (and thus alignment) and the size of the object.

Assuming all the above is handled and does not change. The above code is very fragile to any change. It assumes all the data is in the object. This is usually not the case (think of a string, most of the data is at the other end of a pointer). So just because you serialize the memory occupied by an object does not always mean you get all the data that belongs to an object.

To use an optimal binary serialization. I would serialize each member individually. That way at least you avoid the issues with packing introduced by the compiler.

Design

Not sure what m_buildingQualityDecimals is used for or why it is useful. If you are worried about the exactness of floating point then you should retain the data as a string or integer (depending on the situation).

Never make assumptions about a class

    file.write((char*) &b, sizeof(b)); // writing file to binary by casting to a char*

Here the external user is serializing the class. But has no idea what is inside the class. This makes this code impossibly brittle to change. It will almost certainly break over time.

What you should do is ask the object to serialize itself to the stream. The object understands itself and its members. So it can optimally serialize itself to the stream.

       b.write(file); // b can serialize itself to the stream.

  class Building
  {
     // Stuff
     void write(std::ostream& out)
     {
         char version = 1;
         out.write(&version, 1);
         out.write(&m_age, sizeof(m_age));
         out.write(m_locationCode, 7);
         out.write(&m_buildingQuality, sizeof(m_buildingQuality));
      }

The same applies to the read.

  file2.read((char *) b2, sizeof(*b2));
  // I would write...
  b2->read(file);

Normal serialization

// Testing process
std::cout << "Age: " << b2->getAge() << "; Loc: " << b2->getLoc() <<
            "; Building Quality:" <<  std::setprecision(b2->getDecimals()) << b2->getQuality() <<std::endl;

Again you are providing geter methods to access the member and retrieve values that you then use to perform an action. Geter methods break encapsulation and expose the internal types of the object. You should never use them like this. What you should do is ask the class to serialize itself to the stream. There is even a standard idiom for this in C++ and it is called the output operator.

  class Building
  {
       friend std::ostream& operator<<(std::ostream& out, Building const& value)
       {
           return out << "Age: " << value.m_age 
                      << "; Loc: " << value.m_locationCode 
                      << "; Building Quality:" << value.m_buildingQuality
                      << "\n";
        }

Prefer "\n" over std::endl

The only difference between the two is that std::endl also flushes the stream. The stream will flush itself automatically when it needs to and thus forcing a flush is only going to make it less efficient. Humans are notoriously bad at making this decision; the code is actually very good at. So let the code decide when to flush.

Don't use pointers in C++

    Building* b2 = new Building(0, "000-000", "0.0");

    // what happens if an exception happens between these two points?

    delete b2;

Using pointers is very C like and its not done much in modern C++ (unless you are implementing a container class or smart pointer or trying to do some tricky low level optimization).

In normal code you very rarely see them. The problem is that they do not show ownership semantics (the owner of a pointer is responsible for calling delete on it). So it is not clear who should delete them. So in most cases normal variables are a preferred and if you must dynamically create an object then you should be using a smart pointer so that the ownership semantics are specified.

In this case a normal object is best.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.