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

Reading and writing binary data in C++

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

Problem

I have written a small container class which groups a 3D position, a normal vector and a texture coordinate into one object. It uses the glm library for the actual data types (vec2 and vec3). This class is named Vertex. Here is the source:

Vertex.h

#ifndef CPP3D_VERTEX_H
#define CPP3D_VERTEX_H

#include 
#include 

namespace cpp3d {

class Vertex {

public:
    glm::vec3 position;
    glm::vec3 normal;
    glm::vec2 texCoord;
    Vertex();
    Vertex(const glm::vec3& position);
    Vertex(const glm::vec3& position, const glm::vec3& normal);
    Vertex(const glm::vec3& position, const glm::vec3& normal, const glm::vec2& texCoord);
    bool operator==(const Vertex& other);
};

std::ostream& operator>(std::istream &stream, Vertex& vertex);

}

#endif


Vertex.cc

#include "Vertex.h"

namespace cpp3d {

using namespace glm;
using namespace std;

Vertex::Vertex() {
}

Vertex::Vertex(const vec3& position) :
        position(position) {
}

Vertex::Vertex(const vec3& position, const vec3& normal) :
        position(position), normal(normal) {
}

Vertex::Vertex(const vec3& position, const vec3& normal, const vec2& texCoord) :
        position(position), normal(normal), texCoord(texCoord) {
}

bool Vertex::operator ==(const Vertex& other) {
    return position == other.position && normal == other.normal && texCoord == other.texCoord;
}

ostream& operator(&vertex), sizeof(vertex));
    return stream;
}

istream& operator>>(istream &stream, Vertex& vertex) {
    stream.read(reinterpret_cast(&vertex), sizeof(vertex));
    return stream;
}

}


As you can see I've overloaded the stream operators to write and read the data the object contains. For this I simply dump the whole object which results in writing eight float numbers so the output will be 32 bytes. I know that this is a very simple file format but I want to read the data as fast as possible so I don't want to parse text files. I also don't really care about different endiannesses (unless it can be imple

Solution

-
Both std::istream and std::ostream are actually not defined in `, despite the fact that they look similar. They're respectively defined in and .

You should instead include these in the implementation file and include
(the iostream declaration forwarding library) in the header.

-
These data members should be
private:

glm::vec3 position;
glm::vec3 normal;
glm::vec2 texCoord;


Otherwise, they will be accessible outside of the class. If they are not meant to be this way, then you should use a
struct instead, which are public by default.

-
If you don't need a default constructor, then leave it out. The compiler will provide one for you.

-
Regarding
operator==:

Conditional operators overloads, such as
operator==, should be const` as they should not modify any data members:

bool operator==(const Vertex& other) const;


Its definition could also be split into multiple lines, which is easier to read than one long line:

bool Vertex::operator==(const Vertex& other) {
    return position == other.position
        && normal == other.normal
        && texCoord == other.texCoord;
}

Code Snippets

glm::vec3 position;
glm::vec3 normal;
glm::vec2 texCoord;
bool operator==(const Vertex& other) const;
bool Vertex::operator==(const Vertex& other) {
    return position == other.position
        && normal == other.normal
        && texCoord == other.texCoord;
}

Context

StackExchange Code Review Q#55803, answer score: 6

Revisions (0)

No revisions yet.