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

Serializable Queue header only library

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

Problem

I was hoping someone could check the library I 'm working on to see if they could give any suggestions. My biggest issue right now is getting tuple to work with more than just the basic types and I also wanted to add exception safety; is this only necessary for the file save/load functions? I also still need to add the rule of 5 for this.

https://github.com/Salgat/SerializeQueue

```
//=====================================================================================================================================
// The MIT License (MIT)
// Copyright (c) 2015 Austin Salgat
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files
// (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify,
// merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished
// to do so, subject to the following conditions:
// The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//=====================================================================================================================================

#pragma once
#ifndef __SERIALIZE_QUEUE__
#define __SERIALIZE_QUEUE__

#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include

namespace

Solution

Three things that stand out to me:

Returning by const value is detrimental to performance:

Probably the main issue I've spotted in your code. In a place such as this:

inline int const pop(tag) { ...
           ^^^^^


The extra const is harmless, though it might produce a compiler warning depending on platform and settings, it will have no negative side effects.

When you are returning a complex object, however, such as in here:

inline std::vector const pop(tag>) { ...
                      ^^^^^


You are implicitly throwing away the ability for the compiler to move the return value and elide the copy. A const-qualified object cannot be moved, because moveing alters the source object (makes it empty). In that case, you are likely to pay for a full deep copy of the vector just because it was declared as a const value.

Also refer to this StackOverflow thread for more in-depth details on this specifically.

That being said, const is still very important to ensure single-assignment and on read-only input parameters. Return-by-value is probably the only place where it is now advised against its use.

No need for inline:

When a method is already declared inside the body of a class, directly in the header file, inline is implicit (and also explicit to the reader, for obvious reasons). There is not actual need to supply the keyword in such cases.

Names starting with __ (double underscore) bare a restriction:

__SERIALIZE_QUEUE__ qualifies as an identifier using a notation reserved for the implementation and/or standard library. Simply using SERIALIZE_QUEUE_H fixes this.

Code Snippets

inline int const pop(tag<int>) { ...
           ^^^^^
inline std::vector<T> const pop(tag<std::vector<T>>) { ...
                      ^^^^^

Context

StackExchange Code Review Q#92845, answer score: 5

Revisions (0)

No revisions yet.