Skip to content

Support explicit insertion of boost::none #48

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

Closed
bjoernpollex opened this issue May 4, 2016 · 16 comments
Closed

Support explicit insertion of boost::none #48

bjoernpollex opened this issue May 4, 2016 · 16 comments

Comments

@bjoernpollex
Copy link

It would be useful to support explicitly inserting NULL values by writing boost::none, like this:

db << "INSERT INTO foo(field) VALUES (?)" << boost::none;

Currently this does not work because boost::none is a special type, and not automatically converted to any of the overloads for boost::optional.

@Killili
Copy link
Collaborator

Killili commented May 4, 2016

Sounds interessting, never thought about that way to do it.
But i would like to have that in the library without having a new Boost dependent feature.

db << "INSERT INTO foo(field) VALUES (?)" << sqlite3::null;

This would be a nice thing to have.

@bjoernpollex
Copy link
Author

Sure, that works just as well.

@Killili
Copy link
Collaborator

Killili commented May 6, 2016

I added that as a extension because it has limited use to the normal user and does not require changes to the main header to work.
See pull request #51 for source and example.

First i tried to make the null_value more intelligent and able to store the data is the value was not null but that would duplicate the functionality boost::optional already has and we have support for that already. Thats why this is such a simplistic approach.

@aminroosta if you are happy with this too, plz pull.

@aminroosta
Copy link
Collaborator

@Killili Thanks for #51,i have a different approach in my mind. take a look at #52 and then we will choose which one to go forward with.

@Killili
Copy link
Collaborator

Killili commented May 6, 2016

Thats elegant and gets rid of boost::optional in one go, i like it.

@aminroosta
Copy link
Collaborator

Cool, i will update readme in #52 and then merge.

@bjoernpollex
Copy link
Author

bjoernpollex commented May 18, 2016

Hm, I agree that this solution is quite elegant, but deprecating boost::optional doesn't seem optimal to me. std::unique_ptr does dynamic memory allocation, which boost::optional does not. boost::optional also has other useful functions, such as delegating assignment and equality comparison operators. I'd prefer to keep it in. At some point this library can maybe deprecate it in favor of the upcoming std::optional.

@aminroosta
Copy link
Collaborator

Is std::optional coming to C++17 ?

@Killili
Copy link
Collaborator

Killili commented May 18, 2016

So we deprecate boost but let it in till then, but its non the less deprecated.

@arude
Copy link

arude commented May 18, 2016

std::optional is part of Fundamentals v1 which was done about 1 year ago which suggests that it should make it into C++17.

@aminroosta
Copy link
Collaborator

aminroosta commented May 18, 2016

If std::optional is going to be part of C++17 then i think we should deprecate both boost and unique_ptr approaches for a while and eventually drop them when we migrate to only supporting C++17

@Killili
Copy link
Collaborator

Killili commented May 18, 2016

I dont see why we should depricate the unique_ptr stuff its a valid alternative to the optional stuff. And i would prefer it over the other. But we might add to the deprecation text of boost::optional that it will be replaced by std::optional when its available.

@aminroosta
Copy link
Collaborator

aminroosta commented May 18, 2016

According to std::optional reference.

Any instance of optional<T> at any given point in time
either contains a value or does not contain a value.
If an optional contains a value, the value is guaranteed
to be allocated as part of the optional object footprint,
i.e. no dynamic memory allocation ever takes place
.
Thus, an optional object models an object, not a pointer,
even though the operator*() and operator->() are defined.

@Killili
Copy link
Collaborator

Killili commented May 18, 2016

Thats what @bjoernpollex said. And he has a valid point in that its usefull and should be in there. But i dont fear a simple new ;). On the other hand thats true for unique_ptr, too if you think about it. The space for the pointer is allocated at instanciation and might never contain anything other then the nullptr ;)

@aminroosta
Copy link
Collaborator

I'm happy with the way we are handling null values right now. :-)
Lets come back to this when we actually have a compiler which supports std::optional<T>.

@bjoernpollex
Copy link
Author

I agree, right now both ways work and are useful for different scenarios.

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

4 participants