C++ getaddrinfo() wrapper

EDIT: New version can be found here: C++ getaddrinfo()/addrinfo wrapper (rewrite)

I'm a C++ novice, but an experienced Python and C programmer. I decided to code a C++ wrapper around getaddrinfo() to get my feet wet (targeted at Unix, specifically). Am I using good style/best practises? How can I improve this?

```
#include
#include
#include
#include

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

class GetAddrInfo
{
private:
addrinfo *result;
const addrinfo hints;
public:
const std::string node;
const std::string service;

explicit GetAddrInfo(const std::string &node_,
const std::string &service_, const addrinfo &hints_) :
hints(hints_), node(node_), service(service_)
{
int res = getaddrinfo(node.c_str(), service.c_str(),
&hints, &result);
if(res != 0)
throw std::system_error(res, std::generic_category(),
gai_strerror(res));
}

GetAddrInfo(GetAddrInfo &&other) :
result(std::move(other.result)), hints(std::move(other.hints)),
node(std::move(other.node)), service(std::move(other.service))
{
// The other object's dead
other.result = nullptr;
}

GetAddrInfo(const GetAddrInfo &other) :
GetAddrInfo(other.node, other.service, other.hints) {}

~GetAddrInfo()
{
if(result != nullptr)
freeaddrinfo(result);
}

class iterator
{
private:
addrinfo *rp;
public:
using self_type = iterator;
using difference_type = std::ptrdiff_t;
using value_type = addrinfo;
using pointer = addrinfo *;
using reference = addrinfo &;
using iterator_category = std::forwar

Operators

You don't need to override operator= in the iterator - the default will do what you want just fine. In general, remember the rules of 3, 5, and 0. This also means that you need to implement the copy and move assignment operators for your AddressInfo class. I'd also recommend adding an explicit conversion to and from an addrinfo* so users of the library can still interface with raw uses of it.

Your copy constructor should absolutely not call the parameterized constructor - getaddrinfo is usually an expensive call, and it'll be much cheaper to just copy the addrinfo* manually.

Your operator++ shouldn't do any bounds checking - its the caller's job to make sure that incrementing is a valid operation.

Iterator

Your iterator can be templated on the type so that you can then do

template 
struct Iterator { ... };

using iterator = Iterator;
using const_iterator = Iterator;

You make some weird choices in your operator overloading and implementation of your operators (as mentioned above). You can just specify all constructors as default as well. I'd make it a struct instead of a class, but that's just personal preference.

Interface

You add some weird member functions that I don't think add any value - to_container and to_container_pair. If a user wants to convert your thing to some other container, they already have the ability to do so. to_container adds very little value.

to_container_pair adds a bit more value (it does quite a bit more work), but there are a few criticisms I have for it.

You implemented your iterator but you aren't using it - that's silly. You also are doing too much in this function. I'd pull out getting the socket pointer to a function, and getting the ip address to another function. Then you can do it in a more iterator based way.

template  class T>
T to_container_pair()
{
    T container;

    std::transform(begin(), end(), std::back_inserter(container), 
      [](const addrinfo& address) {
        return std::make_pair(address.ai_family, getIpAddress(address));
      });

    return container;
}

std::string getIpAddress(const addrinfo& address)
{
    char str_addr[INET6_ADDRSTRLEN];
    void* sock_ptr = getSocketPointer(address);
    inet_ntop(address.ai_family, sock_ptr, str_addr, sizeof(str_addr));
    return std::string(str_addr);    
}

void* getSocketPointer(const addrinfo& address)
{
    if (address.ai_family == AF_INET)
    {
        return &(static_cast(address.ai_addr)->sin_addr);
    } 
    else
    {
        return &(static_cast(address.ai_addr)->sin6_addr);
    }    
}

Move construction

For primitives you generally don't need to move them - they fit into registers and are essentially free to copy. Thus you don't need to move result.

Names/types

In C++ we generally call a pointer to something a T p and not T p - the second version is a C thing. You can also benefit by using much more expressive names - result becomes addresses, rp becomes current or currentAddress, etc.

The name GetAddrInfo isn't really how you want to name a class. You could name a function like that, but the class is really just the address information. You could call it AddrInfo, but there isn't any value to dropping three characters, so AddressInfo is probably best.

addrinfo*

Lastly, I'd consider making a wrapper struct to encapsulate an addrinfo - instead of directly exposing that to a user, who could then mess with the pointers and stuff, give them a struct to use. This can also have better names, because while the names used have been around forever, they're not always descriptive and are a pain to use. If they want access to the raw addrinfo* then they should use the explicit conversion operator you give them.

StackExchange Code Review Q#135708, answer score: 4