Skip to main content
gamedev.net
🔒 Locked

boost::optional For Pointers

Unity
Started by Shinkage 38 replies 10.3k views
Original Post
Shinkage
Shinkage
I thought I'd pose a quick question to the community here to get some feedback on a convention I'm using in a side project: using boost::optional to wrap pointers where a non-exceptional invalid state is expected. In other words, I'm basically pretending null pointers don't exist and using optional wherever you would otherwise be testing for a null pointer.

So far even just using my own code it's helped me avoid a few dumb mistakes, but I can't help but think it might really annoy some C++ programmers out there. So, does anybody out there think this convention is overly cumbersome/excessive? Or even more importantly, counterproductive in some way?
Antheus
Antheus
A rose by any other name....

What do you return instead of null pointer?

In what way is this better than NullObject?
cache_hit
cache_hit
I don't get it. What's wrong with null? In any case, what's your expected result if you do the following:

if (foo){}
Shinkage
Shinkage
I guess I didn't explain the motivation behind doing things this way. There are really two reasons: consistency with non-pointer types, and hinting good practice to the programmer. For the first, the same software has several instances where optionals are used elsewhere for types which don't have an inherent null value, and just using optional for ALL types makes the syntax consistent throughout the program. For the second, seeing optional<pointer> just simply makes it a lot more obvious that, "uninitialized values are a very real possibility here--account for them."

Part of what I'm playing around with in this side project is how far I can go with actually enforcing good usage practices before it just gets annoying. Trying to find that line where it goes from, "this helps me not make mistakes," to, "this is just annoying."

Quote:
Original post by Antheus
In what way is this better than NullObject?


It's no better because that's precisely what it is. It's just an implementation that makes it semantically obvious exactly what's going on. I guess I should mention that there are places in the same library where null pointers will never be returned during non-exceptional execution, and I'm testing this out to make the distinction of where this is more obvious.
jonathanjansson
jonathanjansson
It seems a bit awkward to wrap a pointer in a boost::optional. Isn't the syntax for reading a member going to be (*object)->member? There is probably some overhead as well because to make boost::oprional work with ints and other variables it must store a bool variable somewhere to indicates wether it is set or not set.

I think a better choise is to make a simple smart pointer. Take boost::intrusive_pointer and strip out reference counting from it and call it something like non_null_ptr<T>.

You may also use refreences to show that an object is not null. Although I use it mostly for simple data carrying objects. When passing something like a texture which is usually passed by pointer (or smart pointer) or some object which may be stored for later, passing by reference feels wrong.
Shinkage
Shinkage
Quote:
Original post by jonathanjansson
It seems a bit awkward to wrap a pointer in a boost::optional. Isn't the syntax for reading a member going to be (*object)->member? There is probably some overhead as well because to make boost::oprional work with ints and other variables it must store a bool variable somewhere to indicates wether it is set or not set.

I think a better choise is to make a simple smart pointer. Take boost::intrusive_pointer and strip out reference counting from it and call it something like non_null_ptr<T>.

You may also use refreences to show that an object is not null. Although I use it mostly for simple data carrying objects. When passing something like a texture which is usually passed by pointer (or smart pointer) or some object which may be stored for later, passing by reference feels wrong.


Well, everything is already wrapped in smart pointers, and I'd like to stick to just one pointer type for consistency sake. If I do end up going with this, I'll probably do some kind of specialization of optional for my specific smart pointer type so that it only takes a single dereference to get at the pointee.

Using references like that wouldn't really work since the memory in question is allocated from the freestore and being managed with smart pointers. I'd have to make some kind of funky smart pointer that deals in references rather than pointers, which would just be strange.
Zahlman
Zahlman
Quote:
Original post by Antheus
What do you return instead of null pointer?


A default-constructed boost::optional<T*>, of course.

Quote:
In what way is this better than NullObject?


I don't know. In what ways is an implementation of a pattern better than the pattern?

The point of what OP is doing is simply to make explicit the concept of nullness of pointers. Instead of pointers having a magical value that indicates that they don't really point at anything, the "optional" pointer is conceptually simply not there.

I don't like it, though. We're adding overhead (boost::optional<T> adds extra data for a flag to indicate whether the underlying data actually represents a T instance - even if T is a pointer, AFAIK), potentially duplicating our signal (since we can still have a NULL pointer in addition to a non-pointer), and spurning one of the best-established idioms of the language.
Sneftel
Sneftel
Yeah, this seems like a foolish consistency. Pointers are already optional, and boost::optional is designed to mimic the syntax of null/not-null pointers.

If you really feel like getting your boost::optional rocks off, how 'bout passing around optional references? Those are even more like pointers, and without the weird two-kinds-of-null thing you've got going on there.
cache_hit
cache_hit
Quote:
Original post by Zahlman
Quote:
Original post by Antheus
What do you return instead of null pointer?


A default-constructed boost::optional<T*>, of course.

Quote:
In what way is this better than NullObject?


I don't know. In what ways is an implementation of a pattern better than the pattern?

The point of what OP is doing is simply to make explicit the concept of nullness of pointers. Instead of pointers having a magical value that indicates that they don't really point at anything, the "optional" pointer is conceptually simply not there.

I don't like it, though. We're adding overhead (boost::optional<T> adds extra data for a flag to indicate whether the underlying data actually represents a T instance - even if T is a pointer, AFAIK), potentially duplicating our signal (since we can still have a NULL pointer in addition to a non-pointer), and spurning one of the best-established idioms of the language.


When I think about it some more, I think maybe the OP wants to treat NULL (i.e. the actual built-in pointer NULL value) differently from this other value. In essence, he doesn't want NULL to be a special value which conceptually means "no value", but rather he wants NULL to be "just another legitimate value like anything else". So just using NULL for everything doesn't even work for what he wants.

That being said, it makes me wonder if there's some other kind of design problem going on. In any case, I cringe at this idea, but it's hard to come up with something better without being given an actual problem to solve.
davepermen
davepermen
I like his idea, and understand his two reasons. the consistency thing is obvious. the other one might not.

there are things that should never fail, but might. those return a 0 pointer. there are things, that are expected to fail, those are then in optional. i get his idea.

Model* mainCharacter = content->Load("Main Character.mesh");
as this should not error. if it does, it causes an exception (normally)

Server* connection = connectToMultiplayerServer();
this can be valid to not return something, as it is valid to not have an internet connection.

optional<Server*> connection = connectToMultiplayerServer();
makes sense there. the optional shows if there's a connection, or not. null can not happen, as this would have thrown before.

optional<Object*> hit = World.FindCollision(mainCharacter);
i'd use it in such cases quite often.

ads some clarity to the code, that, indeed, the return value is optional, and not expected.
If that's not the help you're after then you're going to have to explain the problem better than what you have. - joanusdmentia
My Page davepermen.net | My Music on Bandcamp and on Soundcloud
Zipster
Zipster
So if I get this straight, your intention is to indicate to the user which pointers they need to check for null, since some pointers are null during normal execution and must be checked, whereas others are only null in exceptional circumstances (in which case an exception would have been thrown and there's no reason to check the pointer anyway). If this is the case, wouldn't a simple typedef serve the same purpose?

typedef Foo* NullableFooPtr; // Can be NULL, check!
typedef Foo* NonNullableFooPtr; // Can't be NULL if you get far enough to have one, no need to check
davepermen
davepermen
yes that's his intend. together with the consistency thing, you don't have ptrs you have to check, and ptrs you don't have to, and non-pointer types you don't have to, and non-pointer-with-additional-flag that you have to.

if it's optional<T>, you have to check. else, you never have to (except with some form of exception handling for the extreme cases, but never in ordinary code flow)
If that's not the help you're after then you're going to have to explain the problem better than what you have. - joanusdmentia
My Page davepermen.net | My Music on Bandcamp and on Soundcloud
cache_hit
cache_hit
Quote:
Original post by Zipster
So if I get this straight, your intention is to indicate to the user which pointers they need to check for null, since some pointers are null during normal execution and must be checked, whereas others are only null in exceptional circumstances (in which case an exception would have been thrown and there's no reason to check the pointer anyway). If this is the case, wouldn't a simple typedef serve the same purpose?

typedef Foo* NullableFooPtr; // Can be NULL, check!
typedef Foo* NonNullableFooPtr; // Can't be NULL if you get far enough to have one, no need to check


I think a better way to indicate this is simply with a comment, personally. If there's one thing I've learned over the past few years, it's that pushing all problem solutions through a boostify_if_possible filter is not the answer, and you will regret it later on. Granted, your solution had nothing to do with boost, but it also just seems a little silly. Should a non-nullable pointer be convertible to a nullable pointer? Seems like it. What about vice versa? Ooops, probably not.

I mean, really. KISS. If some pointers should never be null, document that with a comment on the function that returns the pointer. The older I get, the more I realize that sometimes you just need to stop creating new problems and just GSD.
Zipster
Zipster
Quote:
Original post by cache_hit
I think a better way to indicate this is simply with a comment, personally. If there's one thing I've learned over the past few years, it's that pushing all problem solutions through a boostify_if_possible filter is not the answer, and you will regret it later on. Granted, your solution had nothing to do with boost, but it also just seems a little silly. Should a non-nullable pointer be convertible to a nullable pointer? Seems like it. What about vice versa? Ooops, probably not.

I never said it was a good idea, I was only proposing an alternative that made the code more self-documenting like the OP wanted while using a basic feature of the language that introduces zero overhead, as opposed to tacking on additional libraries. I would personally use proper documentation as well.
owl
owl
Quote:
Original post by Zipster
typedef Foo* NullableFooPtr; // Can be NULL, check!
typedef Foo* NonNullableFooPtr; // Can't be NULL if you get far enough to have one, no need to check


I'm trying, but I can't see the usefulness of this... I mean, why NonNullableFooPtr will be non-nullable at all?

If it's just a sort of reminder at the time of declaring the variable I'd rather define the actual variable as:

Foo* NonNulablePointerVariableToFoo;
[size="2"]I like the Walrus best.
Codarki
Codarki
I return references when it can't be null. I like boost::optional in concept, for example returning optional bounding box for a mesh which might be dynamic mesh without any geometry yet.

But I really dislike all the headers boost nowdays includes. Workarounds for every possible compiler, and optional.hpp includes half (or whole) mpl. I believe it was around 700 headers for just that. If you have around thousand source files, it does slow down the compilation and link time (alot of duplicate symbols). I've injected some predeclarations into boost namespace, but that doesnt feel right. Btw try include boost::format and put "Show Includes" on if you're with MSVC.

I'm happy we have now std smart pointers rammed into memory header, compared to boost it includes only 1/5th headers.
Codarki
Codarki
More on topic, don't return null pointers. IMO, try to make all functions no-fail or return null objects as said before. Asserts and exceptions (or abort()) will handle the error cases. It makes alot cleaner syntax.

Although there are exceptions. At very low-level code it might be handy to use pointer aritmethic and code closer to C, but that should be implementation details.
Shinkage
Shinkage
Dave's pretty much got exactly what my intent and reasoning are. The one thing to keep in mind when suggesting an alternative is that consistency is, to me at least, as important a goal as hinting at null-ability. Specifically, non-pointer types can use optional in exactly the same way. References would be an option (maybe) if this didn't get used in factory functions and the objects weren't managed by smart pointers. I suppose introducing a new smart pointer type that holds references would be another option, but I'm not sure it meets my consistency criterion as well.
Quote:
Original post by Zipster
typedef Foo* NullableFooPtr; // Can be NULL, check!
typedef Foo* NonNullableFooPtr; // Can't be NULL if you get far enough to have one, no need to check

I don't like this solution because it seems to be saying that null-ability is a property of the pointer, as opposed to being a property of its usage. I think the semantics of optional<FooPtr> are closer to what's really going on, but that may just be me.
Quote:
Original post by cache_hit
I mean, really. KISS. If some pointers should never be null, document that with a comment on the function that returns the pointer. The older I get, the more I realize that sometimes you just need to stop creating new problems and just GSD.

That's the thing, optional<FooPtr> effectively is the documentation. Not only that, it's a form of documentation that's always visible to the user. No need to check the docs for every function, or remember any of that--it's implicit and unavoidable in the code itself. The motivation being that this is one of this bits of documentation that's often quite easy to miss and just as easy to forget to abide by.
cache_hit
cache_hit
Quote:
Original post by Shinkage
Dave's pretty much got exactly what my intent and reasoning are. The one thing to keep in mind when suggesting an alternative is that consistency is, to me at least, as important a goal as hinting at null-ability. Specifically, non-pointer types can use optional in exactly the same way. References would be an option (maybe) if this didn't get used in factory functions and the objects weren't managed by smart pointers. I suppose introducing a new smart pointer type that holds references would be another option, but I'm not sure it meets my consistency criterion as well.
Quote:
Original post by Zipster
typedef Foo* NullableFooPtr; // Can be NULL, check!
typedef Foo* NonNullableFooPtr; // Can't be NULL if you get far enough to have one, no need to check

I don't like this solution because it seems to be saying that null-ability is a property of the pointer, as opposed to being a property of its usage. I think the semantics of optional<FooPtr> are closer to what's really going on, but that may just be me.
Quote:
Original post by cache_hit
I mean, really. KISS. If some pointers should never be null, document that with a comment on the function that returns the pointer. The older I get, the more I realize that sometimes you just need to stop creating new problems and just GSD.

That's the thing, optional<FooPtr> effectively is the documentation. Not only that, it's a form of documentation that's always visible to the user. No need to check the docs for every function, or remember any of that--it's implicit and unavoidable in the code itself. The motivation being that this is one of this bits of documentation that's often quite easy to miss and just as easy to forget to abide by.


I get it, trust me. I just think that simplicity is a far more noble goal to strive for than consistency.

Anyway, what does the following code snippet print?

optional<Foo> f;optional<Foo> g(NULL);if (f){   cout << "1" << endl;}else if (g){   cout << "2" << endl;}else{   cout << "3" << endl;}

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.