Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move constructor for pqxx::connection #113

Closed
hectorhon opened this issue Jul 18, 2018 · 9 comments
Closed

Move constructor for pqxx::connection #113

hectorhon opened this issue Jul 18, 2018 · 9 comments
Milestone

Comments

@hectorhon
Copy link

Are there plans to add move constructors for the connection classes? This would allow the connections to be stored in a vector, for example.

Thanks for the great work!

@jtv
Copy link
Owner

jtv commented Jul 19, 2018

I looked into this a few years ago, but there were some complications. Not necessarily blockers, but adding move constructors may require some changes across the inheritance hierarchy.

In particular, a concrete connection type contains a policy object, and derives from connection_base which gets a reference to that object. It'd be great to find a better design for that whole construct, but the change to the inheritance hierarchy would be profound. If anybody out there defined their own connection types they could be severely affected.

Which is not to say that the idea is doomed. It just needs careful thought and discussion, and it's good to have this issue as an impulse for that.

@hectorhon
Copy link
Author

Is there any specific reason connection_base needs to take a reference to the policy object? Other than compatibility issues, would it be a good idea if I moved m_policy from the derived classes to the base class?

@jtv
Copy link
Owner

jtv commented Jul 23, 2018

The policy reference is in the base class. Storing the policy object is more of a problem. We'd have to have a single class template, with the connection policy as a template parameter. That would be nice and simple, but different instantiations of the template would no longer be related! Every mention of a connection class anywhere in client code would have to specify the exact type of connection, or be templated on the connection type.

@KayEss
Copy link
Contributor

KayEss commented Jul 23, 2018

If you store a unique_ptr to it (pimpl style) does that not solve the problem and allow the type to be movable?

@jtv
Copy link
Owner

jtv commented Jul 23, 2018

True. Hate to do an avoidable heap allocation, but otherwise I think that would work.

@jtv jtv added this to the 7.0 milestone Sep 4, 2018
@jtv
Copy link
Owner

jtv commented Sep 4, 2018

I made this a candidate for a prospective new major release.

@hectorhon
Copy link
Author

Thanks Jeroen! Currently I've been working around the issue by allocating into an std::array instead, the only downside is the size will be fixed. But that's OK for a basic connection pool :)

@jtv
Copy link
Owner

jtv commented Sep 4, 2018

Glad to hear that you're not stuck for now. But yes, maybe it's time to be a bit more wasteful with resources and a bit more considerate towards the programmer.

@jtv
Copy link
Owner

jtv commented Aug 27, 2019

Finally added a move constructor for connection! No move assignment as yet, and there are some safety restrictions.

@jtv jtv closed this as completed Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants