Sign in to follow this  
jonathanplumb

Function Pointer as argument nightmare ('&' requires l-value)

Recommended Posts

As the title states, I'm getting a "'&' requires l-value" error from the following code:
void InitTitleScreen();

void CSystem::PlayMovie(const char *filename, void *func(void))
{
	//TODO: Play movie ....

         //Call the end-function at the end of the movie
	*func();
}

void CSystem::GameLoop()
{
	switch (g_nGameState)
	{
	case STATE_INITIALIZATION:
		{
			PlayMovie("data\\Intro.mov", &InitTitleScreen());
		}
		break;
	}
}

void InitTitleScreen()
{
	MessageBox(NULL, L"TEST", L"Function pointers suck...", MB_OK);
}
The line that is giving me the error is:
PlayMovie("data\\Intro.mov", &InitTitleScreen());
I've read and re-read tutorials on function pointers ... and can easily do it in C#, but I want to do it in C++ and it is just a little frustrating. The way I'm seeing it, &InitTitleScreen() is the address of InitTitleScreen(), and I pass that to the PlayMovie function, which then calls the function through the parameter. I know that the "l-value" thing means that there is no value to assign from that, but that doesn't make sense, because shouldn't the function have an address? I also tried doing the following:
void *FuncPtr = &InitTitleScreen;
but that didn't help me much either ... just more errors...

Share this post


Link to post
Share on other sites
Quote:
Original post by jonathanplumb
The way I'm seeing it, &InitTitleScreen() is the address of InitTitleScreen()
InitTitleScreen() is not a function. InitTitleScreen() is a function call. InitTitleScreen is a function.

Share this post


Link to post
Share on other sites
Quote:
Original post by Hodgman
Try replacing "void *func(void)" with "void (*func)(void)"
Actually, I'd recommend changing it to boost::function<void ()> - it's more readable (in my opinion) and it's more flexible, too!

Share this post


Link to post
Share on other sites
I'm not sure what that "boost::" is, but I fixed it by doing the following:

Changed PlayMovie() to the following:

void CSystem::PlayMovie(const char *filename, void (*func)(void))
{
//TODO: Play movie ....

(*func)();
}


Changed GameLoop() to the following:

void CSystem::GameLoop()
{
switch (g_nGameState)
{
case STATE_INITIALIZATION:
{
PlayMovie("data\\Intro.mov", &InitTitleScreen);
}
break;
}
}


and InitTitleScreen() didn't change. I think it was just a problem with my parenthesis not in the right place, making some odd typecasts or something.

Thanks for the help. Now off to learn OpenGL --

Share this post


Link to post
Share on other sites
Quote:
Original post by jonathanplumb
I'm not sure what that "boost::" is, but I fixed it by doing the following:


boost is a great big bunch of C++ awesomeness. It's got a whole bunch of neat stuff, including smart pointer classes, cross-platform threading/synchronization, cross-platform file system classes, serialization, networking, regular expressions, and more. Certainly worth checking out.

Codeka was talking about boost::function which lets you create function objects for use as callbacks. You can also combine it with boost::bind to create callbacks to class member functions, which is really neat.

Share this post


Link to post
Share on other sites
Quote:
Original post by jonathanplumb

void CSystem::PlayMovie(const char *filename, void (*func)(void))
{
//TODO: Play movie ....

(*func)();
}


Since you cannot pass functions as arguments (only function pointers), the asterisk in parameters is optional.

It is also optional when you call a function through a function pointer -- in fact, it is even more natural without the asterisk, because calling "normal functions" does a silent conversion to a function pointer first before calling the function anyway -- but that detail is not so important :)

void CSystem::PlayMovie(const char *filename, void func())
{
//TODO: Play movie ....

func();
}

This SHOULD work, but I haven't tested it. Please confirm.

Share this post


Link to post
Share on other sites
Quote:
Actually, I'd recommend changing it to boost::function<void ()> - it's more readable (in my opinion) and it's more flexible, too!

Quote:
... Anyway @OP you should really consider to take a look at boost.


I know GD.net posters are in love with boost. It's a good library, and I have used it many times.

However, I think far too often people here recommend boost for things far to trivial to matter.

Here you're recommending using boost instead of a function pointer.
"boost::function<void ()> func" vs "void func()" or "void (*func)()"
I don't see any advantage, and I see a few disadvantages.
1st: It makes boost a requirement.
2nd: Any programmer than knows boost should first be familiar with function pointers.
3rd: It's actually far more characters, less readable (IMHO), and slower-compiling than a basic function pointer.

If the extra function pointer functionality that boost provides was useful in this situation, I'd have no problem with recommending it. But, here, it's not.

All I want to say is, boost isn't ALWAYS the optimal solution. Don't just recommend it everywhere you can out of habit or whatever.

I'm only saying this because I see this type of thing far too often here.


And again, don't get me wrong, boost is a good library and I use it when I need it.
And OP, don't let this put you off boost, either.

Share this post


Link to post
Share on other sites
Did you know that C++ also allows Function References? Just as you'd expect it's the same difference as between a pointer and a reference, for any other type.
It's not very commonly used but it makes the calling syntax nicer.

Share this post


Link to post
Share on other sites
Quote:
Original post by yacwroy
Here you're recommending using boost instead of a function pointer.
"boost::function<void ()> func" vs "void func()" or "void (*func)()"
I don't see any advantage, and I see a few disadvantages.
1st: It makes boost a requirement.
Boost is a requirement of modern C++, unless you can adequately demonstrate why it is not feasible in your situation (i.e. platform compatibility or corporate restrictions).
Quote:
2nd: Any programmer than knows boost should first be familiar with function pointers.
Every programmer who uses std::map should know how to implement a balanced red-black tree too - but you don't see anyone recommending they learn that first, do you?
Quote:
3rd: It's actually far more characters, less readable (IMHO), and slower-compiling than a basic function pointer.
Less characters? Really? Every programmer should be a proficient typist, *regardless* of all other considerations. Modern IDEs also include both auto-completion and syntax highlighting, as well as modern compilers with decent performance.
Quote:
If the extra function pointer functionality that boost provides was useful in this situation, I'd have no problem with recommending it. But, here, it's not.
Who is to say it won't be useful soon? After all, it is only a matter of time till the OP wants to use a member-function as a callback, and then the function pointer approach dies a horrific death.

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
Quote:
Original post by yacwroy
Here you're recommending using boost instead of a function pointer.
"boost::function<void ()> func" vs "void func()" or "void (*func)()"
I don't see any advantage, and I see a few disadvantages.
1st: It makes boost a requirement.
Boost is a requirement of modern C++, unless you can adequately demonstrate why it is not feasible in your situation (i.e. platform compatibility or corporate restrictions)
No way. Boost provides tools for those that need them. I'd guess less than 1/2 of the C++ apps released this year used boost. It is a million miles away from being a requirement.


Quote:
Quote:
2nd: Any programmer than knows boost should first be familiar with function pointers.

Every programmer who uses std::map should know how to implement a balanced red-black tree too
Eh?
Function pointers are an integral part of C++. Red-black trees are a special case of weighted binary tree that only people who plan on writing their own binary tree might need to know. 99% of coders probably haven't implemented a binary weighted tree themselves. However, the number of C++ users that haven't used function pointers is going to be like 20%, if you exclude all those just past hello world.


Quote:
Quote:
3rd: It's actually far more characters, less readable (IMHO), and slower-compiling than a basic function pointer.
Less characters? Really? Every programmer should be a proficient typist, *regardless* of all other considerations. Modern IDEs also include both auto-completion and syntax highlighting, as well as modern compilers with decent performance.

I didn't mean it was a problem to type. Number of chars is about readability. And in many cases long lines can lead to multiple lines, especially in function declarations.


Quote:
Quote:
If the extra function pointer functionality that boost provides was useful in this situation, I'd have no problem with recommending it. But, here, it's not.
Who is to say it won't be useful soon?

Why not just #include every useful library in existence as it might be useful soon. The fact that it might be useful soon, as many other things might, is no reason for using them now. And it's not useful yet.


Quote:
After all, it is only a matter of time till the OP wants to use a member-function as a callback, and then the function pointer approach dies a horrific death.

Why? Pointers to member functions exist and work fine. And that's all you need for callback. The syntax gets slightly messy, but it's still fewer symbols than the equivalent boost syntax.


And apologies for hijacking the thread and starting what looks like a flame war. I wasn't trying to diss boost, just saying it's not the best answer to every question that it can answer.

Share this post


Link to post
Share on other sites
Quote:
Original post by yacwroy
Quote:
After all, it is only a matter of time till the OP wants to use a member-function as a callback, and then the function pointer approach dies a horrific death.
Why? Pointers to member functions exist and work fine. And that's all you need for callback. The syntax gets slightly messy, but it's still fewer symbols than the equivalent boost syntax.
My point was that you can't define a single function pointer that will accept both free functions and member functions.
Quote:
And apologies for hijacking the thread and starting what looks like a flame war. I wasn't trying to diss boost, just saying it's not the best answer to every question that it can answer.
The bigger issue I was hinting at, which you seem to have missed, is that nobody in this thread actually suggested Boost - let alone that you should rewrite your code to use boost::phoenix.

Try looking in your std library headers - in particular, the header <tr1/functional>.

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
Every programmer who uses std::map should know how to implement a balanced red-black tree too - but you don't see anyone recommending they learn that first, do you?


COME ON! I've used std::map and I don't know what a red-black tree is.

Share this post


Link to post
Share on other sites

Quote:
Original post by swiftcoder
The bigger issue I was hinting at, which you seem to have missed, is that nobody in this thread actually suggested Boost - let alone that you should rewrite your code to use boost::phoenix.


Really... nobody suggested boost in this thread? Have you read the thread?


Quote:
Original post by megamoscha
Anyway @OP you should really consider to take a look at boost.


Quote:
Original post by Codeka
Actually, I'd recommend changing it to boost::function<void ()> - it's more readable (in my opinion) and it's more flexible, too!



Hmm... looks like you haven't. Anyways, I agree, boost in this case just adds confusion, requires learning a new library, will add size and compile time to his project, all for no gain. Not saying boost doesn't have it's place in code, just saying not in all code. Anyways, OP has gotten it working and has moved on. On the contrary, I recommend the OP learn boost at some point, just not for this.

Share this post


Link to post
Share on other sites
Quote:
Original post by m3mb3rsh1p
Quote:
Original post by swiftcoder
Every programmer who uses std::map should know how to implement a balanced red-black tree too - but you don't see anyone recommending they learn that first, do you?
COME ON! I've used std::map and I don't know what a red-black tree is.
That is exactly my point - yacwroy is suggesting that everyone should know how to implement std::tr1::function before they use it, but the same does not hold for other elements of the standard library.
Quote:
Original post by Ready4Dis
Quote:
Original post by swiftcoder
The bigger issue I was hinting at, which you seem to have missed, is that nobody in this thread actually suggested Boost - let alone that you should rewrite your code to use boost::phoenix.
Really... nobody suggested boost in this thread? Have you read the thread?
Have you learned your standard library? In particular, go look in that <tr1/functional> header file I suggested. You will find std::tr1::function - otherwise known as boost::function.

So no, nobody has suggested you use boost. Rather we have suggested that you use an element of the C++ standard library.

If someone had suggested something like boost::phoenix, I probably would have agreed with yacwroy, but for something as basic as boost::function, which has already been added to the C++ standard library, the anti-boost sentiment is ridiculous.

Share this post


Link to post
Share on other sites
Well, I might've technically said "boost::function", but that could just have easily been std::tr1::function. I think that was point that swiftcoder was making.

I also don't see the problem with recommending boost::function (or std::tr1::function) over a simple function pointer. Would you recommend someone use a simple char * until they actually "need" the functionality of std::string?

In general, I would always go the other way around: always prefer the SC++L/boost variant over whatever "raw" equivalent may exist. There's generally no (or so small as to be negligable) overhead, and the benefit in terms of flexibility and robustness are obvious.

Share this post


Link to post
Share on other sites
Much as I love boost, I wouldn't jump to it first here. It is pretty trivial to move from function pointers to boost should the need arise later. The function pointer is the simplest way, and it works.

Boost would be the best solution if the OP wanted to bind member function pointers, because boost::function and boost::bind are better options than hacking your own equivalents.

In the absence of such a requirement, YAGNI says use a function pointer. But for your own sanity use a typedef when working with function pointers [smile].

Share this post


Link to post
Share on other sites
Quote:
Original post by Codeka
I also don't see the problem with recommending boost::function (or std::tr1::function) over a simple function pointer. Would you recommend someone use a simple char * until they actually "need" the functionality of std::string?

That is completely different. Character pointers are dangerous because it is easy to make mistakes when allocating and manipulating them.

Function pointers are relatively hard to mess up in comparison, unless you start trying to cast them.

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
That is exactly my point - yacwroy is suggesting that everyone should know how to implement std::tr1::function before they use it, but the same does not hold for other elements of the standard library.


Aah, I see! At the time, it seemed straightforward that function pointers, as a core part of C++, be understood before using boost::function. I guess it would also be straightforward to use boost::function over function pointers if it was introduced first; just like std::string over char* as someone pointed out. Unfortunately, C++ is usually taught from the ground-up and function pointers, char* and typename* are encountered before std::tr1::function<>, std::string and auto<>...

Share this post


Link to post
Share on other sites
Personally I don't think function pointers are the best solution. If I want typesafety then I use boost::function (or equivalent), if I want utter simplicity then I use a template parameter...

template<typename OnFinishFunction>
void CSystem::PlayMovie(const char * filename, OnFinishFunction finish)
{
load_movie(filename).play();
finish();
}

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
Every programmer who uses std::map should know how to implement a balanced red-black tree too - but you don't see anyone recommending they learn that first, do you?
Quote:
COME ON! I've used std::map and I don't know what a red-black tree is.
That is exactly my point -

yacwroy is suggesting that everyone should know how to implement std::tr1::function before they use it

No I am not.
If you read, I say they should know how to use function pointers. This does in no way imply they should know how to implement a function pointer wrapper class.
Using function pointers is only 1: putting a function's address into a variable, and 2: calling said function from a variable.


Quote:
Original post by swiftcoder
The bigger issue I was hinting at, which you seem to have missed, is that nobody in this thread actually suggested Boost - let alone that you should rewrite your code to use boost::phoenix.
Quote:
Really... nobody suggested boost in this thread? Have you read the thread?
Have you learned your standard library? In particular, go look in that <tr1/functional> header file I suggested. You will find std::tr1::function - otherwise known as boost::function. So no, nobody has suggested you use boost. Rather we have suggested that you use an element of the C++ standard library.

1. When a person says use boost::, they have suggested boost::. Even if it's in std::tr1, they said boost. And nobody here mentioned std::tr1 till you did.
2. std::tr1:: is not std::. It's not part of the standard library yet. It's just a set of candidates that may be added to std:: at some point in the future.
3. Unlikely as it seems, for all you know the std::tr1::function may undergo a syntactic change. Raw function pointers will not. boost:: may even be more concrete than std::tr1::.
Quote:
Original post by wikipediaTR1 is not a standard itself, but rather a draft document. However, most of its proposals are likely to become part of the next official standard.



Quote:
If someone had suggested something like boost::phoenix, I probably would have agreed with yacwroy, but for something as basic as boost::function, which has already been added (me: has not) to the C++ standard library, the anti-boost sentiment is ridiculous.

I think it's pretty unfair to call my comments anti-boost. If I say hammers are useful but you shouldn't go out and buy a hammer just to mash your potatoes or push in your doorstop, It's not anti-hammer sentiment.

Share this post


Link to post
Share on other sites

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