Sign in to follow this  

Unity boost::optional For Pointers

This topic is 2655 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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)

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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;

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by cache_hit
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?

*** Source Snippet Removed ***


It'll print 2, but I think that's beside the point because there's no reasonable use case where you'd end up with the equivalent of optional<ptr>(0), unless you make it yourself... but why would you? I guess we'll have to disagree with regard to simplicity vs. consistency, though. I'm of the mind that I'd rather do something the same way everywhere and have it be slightly more verbose, than do it differently in different contexts and slightly less verbose. Obviously when it gets extreme one way or the other things are different, but I don't think this is that extreme. I also always prefer to have the code be the documentation (in addition to regular documentation) wherever possible.

Two different approaches to programming, I suspect.

Share this post


Link to post
Share on other sites
Quote:
Original post by Shinkage
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.

The semantics of optional<FooPtr> are that the pointer's existence is optional. You've tacked on additional semantics, namely that if a pointer exists you need to check for NULL, and that the lack of boost::optional indicates the pointer is only NULL in exceptional circumstances and you don't need to check. While all that additional meaning may make sense to you it's not something most other people will pick up on immediately (as evidenced by the number of times it's needed to be reiterated), so if your intent is to make the code as clear and self-documenting as possible I think you're going about it the wrong way.

However the concept of "nullable" values has been around in database systems for a while, and proper usage patterns when dealing with these values is quite clear. If something is nullable, it means it can be NULL and you need to check. Otherwise you can assume it isn't. If I understood what you were trying to do correctly, this is one of the most direct ways to convey that notion to the user. It's a property of the value, and usage will follow from that. As for the technical details of conversion, etc., you would need to handle that if you were looking for a formal solution, but if your goal is syntactic sugar then the typedefs should work fine.

Share this post


Link to post
Share on other sites
Quote:
Original post by Shinkage
Quote:
Original post by cache_hit
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?

*** Source Snippet Removed ***


It'll print 2, but I think that's beside the point because there's no reasonable use case where you'd end up with the equivalent of optional<ptr>(0), unless you make it yourself... but why would you? I guess we'll have to disagree with regard to simplicity vs. consistency, though. I'm of the mind that I'd rather do something the same way everywhere and have it be slightly more verbose, than do it differently in different contexts and slightly less verbose. Obviously when it gets extreme one way or the other things are different, but I don't think this is that extreme. I also always prefer to have the code be the documentation (in addition to regular documentation) wherever possible.

Two different approaches to programming, I suspect.


The fact that it does print 2 is one of the biggest reasons I wouldn't want to use it. In that respect it is ANTI consistency with well-established usage practices. I still just don't see anything to gain from doing this. You're trying to circumvent one of the fundamental features of the language and having no tangible benefit to show for it.

I mean I know you probably don't believe me when I say I get it, but I really do. But after abusing boost long enough, you really start to adopt more of a "less is more" philosophy. Happened with me and it happened with everyone else I've known who's used boost heavily.

Share this post


Link to post
Share on other sites
Quote:
I also always prefer to have the code be the documentation (in addition to regular documentation) wherever possible.


X * foo();

This function may return null. There is no need to document it, it's an established fact. Since pointers can be null, the return value may be null.

Quote:
there's no reasonable use case where you'd end up with the equivalent of optional<ptr>(0)


Fallacy: optional prevents returning uninitialized local as per this example:
X * foo() {
X * temp;
if (bar) temp = new X();
return temp;
}

It helps a bit, but doesn't solve the problem by far:
optional<X> * foo() {
X * temp;
if (bar) temp = new X();
return optional<X*>(temp);
}
Now return value is valid, but pointer is not.

Ideally, one would use 'optional<X*> temp;', but this is as reasonable as expecting every variable in C or C++ to be properly initialized.

The above falls very much into reasonable use case, since X is highly likely to be some third-party raw pointer or foo() might be a complex function which will not be able to enforce optional<> in entirety.


While I agree that in ideal world optional<> and other helpers would allow code to be perfectly correct with no possibility of invalid pointers, sooner or later one will touch third-party non-boost code which doesn't use it, thereby breaking the consistency.

It becomes problematic when dealing with C APIs which idiomatically return NULL on failure as well as C++ APIs which use non-throw allocator.

Share this post


Link to post
Share on other sites
Also, it breaks generic programming in the sense that you can no longer treat optional<T*> and T* the same for algorithms and such, when conceptually they are both just pointers. If you have an std::vector<optional<T*> > and an std::vector<T*>, you can't use the same algorithms on them.

Furthermore, in an std::vector<optional<T*>>, the actual pointers aren't adjacent in memory, which will be poor for cache performance. This is in addition to the C interoperability benefits you lose such as those Antheus pointed out.

Ultimately you lose a number of tangible, easily quantifiable benefits, while gaining something that is arguable at best.

Share this post


Link to post
Share on other sites

This topic is 2655 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this