Jump to content

  • Log In with Google      Sign In   
  • Create Account


passing std::shared_ptr to a function?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
25 replies to this topic

#1 jdub   Members   -  Reputation: 419

Like
0Likes
Like

Posted 25 August 2013 - 09:48 PM

I have this code which causes a strange error:

void NBodySim::DrawCSOutput(std::shared_ptr<NBody::Renderer> graphics)
	{
		try
		{
			graphics->SBatch->Begin();

			RECT destRect;
			destRect.left = 0;
			destRect.right = 256;
			destRect.bottom = 256;
			destRect.top = 0;

			graphics->SBatch->Draw(this->uavSrv, destRect);

			graphics->SBatch->End();
		}
		catch(std::exception e)
		{
			const char *wat = e.what();
			int i = 0;
		}
	}

//DrawCSOutput() is called like this:
this->sim.DrawCSOutput(std::shared_ptr<NBody::Renderer>(this->graphics));

//this is the graphics member variable:
Renderer graphics;

I have managed to fix the code by changing the function to take a weak pointer instead but I am wondering, how can I successfully pass std::shared_ptr<T> as an argument to a function?


J.W.

Sponsor:

#2 djtesh   Members   -  Reputation: 243

Like
3Likes
Like

Posted 25 August 2013 - 10:27 PM

The problem with the code above is that after the function call, the shared pointer will end up with 0 references and destroy the object (as it is the ONLY shared pointer to own that object).

 

 

If you would like to pass in a shared_ptr but keep the object alive, store your member as a shared_ptr and then pass it in to the function like so:

void NBodySim::DrawCSOutput(std::shared_ptr<NBody::Renderer> graphics)
	{
		try
		{
			graphics->SBatch->Begin();

			RECT destRect;
			destRect.left = 0;
			destRect.right = 256;
			destRect.bottom = 256;
			destRect.top = 0;

			graphics->SBatch->Draw(this->uavSrv, destRect);

			graphics->SBatch->End();
		}
		catch(std::exception e)
		{
			const char *wat = e.what();
			int i = 0;
		}
	}

//DrawCSOutput() is called like this:
this->sim.DrawCSOutput(this->graphics);

//this is the graphics member variable:
std::shared_ptr<Renderer> graphics;


#3 HappyCoder   Members   -  Reputation: 2608

Like
6Likes
Like

Posted 25 August 2013 - 11:49 PM

I don't think you should be using a shared_ptr in this situation. You are only using the parameter graphics inside that function. A shared pointer is useful for storing a pointer to an object long term, but a local variable shouldn't be a shared pointer unless you are passing it to another function that requires a shared pointer. I would use a reference instead.

void NBodySim::DrawCSOutput(NBody::Renderer& graphics)
	{
		try
		{
			graphics.SBatch->Begin();

			RECT destRect;
			destRect.left = 0;
			destRect.right = 256;
			destRect.bottom = 256;
			destRect.top = 0;

			graphics.SBatch->Draw(this->uavSrv, destRect);

			graphics.SBatch->End();
		}
		catch(std::exception e)
		{
			const char *wat = e.what();
			int i = 0;
		}
	}

//DrawCSOutput() is called like this:
this->sim.DrawCSOutput(this->graphics);

//this is the graphics member variable:
Renderer graphics;


#4 Khatharr   Crossbones+   -  Reputation: 2972

Like
2Likes
Like

Posted 26 August 2013 - 12:33 AM

        catch(std::exception e)
        {
            const char *wat = e.what();
            int i = 0;
        }

 

'wat' indeed.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#5 Servant of the Lord   Crossbones+   -  Reputation: 18739

Like
9Likes
Like

Posted 26 August 2013 - 10:22 AM

By default, pass by const reference, unless you want to write to the original variable also, then pass by reference. Unless that parameter is optional, then pass by pointer.

Unless you want a copy, then pass by value.

 

If the variable is already in a shared pointer, then pass by const reference to shared pointer. Unless you want to modify the variable, in which case you pass by reference to shared_ptr. Unless you want a copy of the shared_ptr (which doesn't copy the variable the pointer points to), then pass a shared_ptr by value.

 

Shared pointers delete the variable they point to, after no more of the shared_ptrs are left alive. To protect yourself from these kind of mistakes, a simple trick is to never create a shared_ptr manually, but instead use std::make_shared() for creating shared_ptrs.

 

But in this situation, all you're wanting to do is make 'NBody::Renderer' accessible to that function. So pass by const reference, if read-only, or regular reference, if read-write. There is no need for a smart pointer to be used here at all.


It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal

[Fly with me on Twitter] [Google+] [My broken website]

[Need web hosting? I personally like A Small Orange]


#6 Khatharr   Crossbones+   -  Reputation: 2972

Like
2Likes
Like

Posted 26 August 2013 - 11:09 AM

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#7 King Mir   Members   -  Reputation: 1968

Like
1Likes
Like

Posted 26 August 2013 - 11:25 AM

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

#8 frob   Moderators   -  Reputation: 20513

Like
4Likes
Like

Posted 26 August 2013 - 11:51 AM

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 

True, but there are also a few minor side effects.

 

First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.

 

Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.

 

 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.


Check out my personal indie blog at bryanwagstaff.com.

#9 Pink Horror   Members   -  Reputation: 1156

Like
2Likes
Like

Posted 26 August 2013 - 01:05 PM

 

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 

True, but there are also a few minor side effects.

 

First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.

 

 

If class X overloads operator->, that affects using the operator-> on a pointer to X?



#10 King Mir   Members   -  Reputation: 1968

Like
1Likes
Like

Posted 26 August 2013 - 02:56 PM

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 
True, but there are also a few minor side effects.
 
First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.
 
Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.
 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.

As pointed out above, overriding -> has no effect on this.

Unless working on a platform where you must use a poor compiler, performing optimizations that the compiler can do for you trivially is premature optimization and should not be a major consideration in program style. A more reasonable objection is that it's verbose, and I think that's the real trade off here. Also using this-> has the advantage that it makes it easy to identify which variables are members.

#11 frob   Moderators   -  Reputation: 20513

Like
1Likes
Like

Posted 26 August 2013 - 04:28 PM

 

 

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 
True, but there are also a few minor side effects.
 
First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.
 
Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.
 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.

 

As pointed out above, overriding -> has no effect on this.

 

 

 
That is what makes it confusing, and one of the reasons to avoid it.
 
The arrow operator is not overriden very often, but it is one of those "gotcha" areas of C++.  It generally works the way programmers expect it to work, but there is some subtlety in the drill-down behavior when operator overloading is involved.
 
Contrived example to demonstrate:

#include <iostream>
 
struct Concrete {
    void foo() { std::cout << "Concrete::foo\n"; }
};
struct Proxy {
    Concrete *a;
    void foo() { std::cout << "Proxy::foo\n"; }
    Concrete* operator->() { return a; }
};
struct Proxy2 {
    Proxy *b;
    void foo() { std::cout << "Proxy2::foo\n"; }
    Proxy& operator->() { return *b; }
 
    void RunFoo() {
        foo(); // Calls Proxy2::foo
        this->foo();  // QUIZ #1:  Which foo() is called?
        (*this).operator->()->foo(); // QUIZ #2: Which foo() is called?
    }
};
int main()
{
    Concrete a;
    Proxy b = { &a };
    Proxy2 c = { &b };
    c.RunFoo();
    c->foo(); // QUIZ #3: Which foo() is called?
}

 
The answers are not surprising at all when you stop to think about them.
 
The problem is that you need to stop and think about them.


Edited by frob, 26 August 2013 - 04:35 PM.
edit: code box got messed up, fixing formatting.

Check out my personal indie blog at bryanwagstaff.com.

#12 King Mir   Members   -  Reputation: 1968

Like
1Likes
Like

Posted 26 August 2013 - 05:21 PM

I don't see why you think that foo() is clearer than this->foo(). To me it's the opposite, because this-> restricts the possible places to look for foo().

There are unintuitive parts to the code above, particularly in how proxy2 acts as a pointer to Concrete, instead of a pointer to Proxy, but that is an unrelated problem.

I guess you could say that the distinction between (*this)->foo() and this->foo() may be easily overlooked, but (*this)->foo() is strange looking enough to warrant special attention. Similarly with (*this).operator->()->foo(), except I don't see why you would ever use that instead of (*this)->foo().

#13 Pink Horror   Members   -  Reputation: 1156

Like
0Likes
Like

Posted 26 August 2013 - 10:25 PM

I guess you could say that the distinction between (*this)->foo() and this->foo() may be easily overlooked, but (*this)->foo() is strange looking enough to warrant special attention. Similarly with (*this).operator->()->foo(), except I don't see why you would ever use that instead of (*this)->foo().

 

I don't know, maybe whenever you want to intentionally make something confusing to try to prove a point.



#14 Trienco   Crossbones+   -  Reputation: 2138

Like
1Likes
Like

Posted 27 August 2013 - 01:54 AM

And yet the fact that operator-> is automatically called recursively until it returns an actual pointer type is occasionally used to implement special behavior or convenient helper functions by using wrapper objects that implement it. It's often the easiest way to keep the calling code clean and blissfully ignorant of implementation details. Try implementing a special proxy on top of a basic proxy to some generic object without exploiting operator->.

 

Basically, the question is: what happened to old-fashioned ways to mark members like 'mMember' or 'member_' that don't have the potential for obscure side effects?

 

There is no ambiguity for functions either, as a function is either a member, global or called through some other object. Global functions should be in a namespace and "using namespace everything" is often preferring laziness over clear code. So suddenly foo() is obviously a member function, as otherwise it would be something like std::foo() or ::foo().


f@dzhttp://festini.device-zero.de

#15 King Mir   Members   -  Reputation: 1968

Like
1Likes
Like

Posted 27 August 2013 - 03:00 AM

And yet the fact that operator-> is automatically called recursively until it returns an actual pointer type is occasionally used to implement special behavior or convenient helper functions by using wrapper objects that implement it. It's often the easiest way to keep the calling code clean and blissfully ignorant of implementation details. Try implementing a special proxy on top of a basic proxy to some generic object without exploiting operator->.
 
Basically, the question is: what happened to old-fashioned ways to mark members like 'mMember' or 'member_' that don't have the potential for obscure side effects?
 
There is no ambiguity for functions either, as a function is either a member, global or called through some other object. Global functions should be in a namespace and "using namespace everything" is often preferring laziness over clear code. So suddenly foo() is obviously a member function, as otherwise it would be something like std::foo() or ::foo().

What's the side effect? this and *this are totally different objects, and anyone who confuses the two should not be using nested proxies.

I also disagree that global functions should always be namespace qualified, because doing so prevents argument dependent lookup. So for example, you should prefer to call unqualified swap, and a using statement rather than calling std::swap.

#16 frob   Moderators   -  Reputation: 20513

Like
3Likes
Like

Posted 27 August 2013 - 11:32 AM

The point is not that it causes ambiguity or strange behavior.

 

The point is that it makes the programmer stop for a moment to figure it out.  

 

Code should be obvious.  You should generally be able to tell at a glance what the code does.  Code that does something tricky should include a great big comment block explaining why it does what it does, the reason the programmer decided to take the less obvious route, and instructions for future developers who maintain it.

 

When I see "this->foo()" my brain needs to pause, if only for a moment, and ask why they are calling a function from the object rather than calling the function directly. 

 

Just the fact that my brain must switch gears for a moment, no matter how small that moment is, that is the issue.


Check out my personal indie blog at bryanwagstaff.com.

#17 King Mir   Members   -  Reputation: 1968

Like
1Likes
Like

Posted 27 August 2013 - 11:55 AM

The only reason your brain needs to pause for a moment is because you're not used to the syntax.

#18 Khatharr   Crossbones+   -  Reputation: 2972

Like
0Likes
Like

Posted 27 August 2013 - 01:53 PM

I already know that it's pointless and that it's annoying as hell. I'm just wondering where it came from.

 

Did people really start doing this just for intellisense? I mean JFC, just upgrade to 2012.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#19 King Mir   Members   -  Reputation: 1968

Like
0Likes
Like

Posted 27 August 2013 - 02:44 PM

I already know that it's pointless and that it's annoying as hell. I'm just wondering where it came from.
 
Did people really start doing this just for intellisense? I mean JFC, just upgrade to 2012.

It's the most compelling reason I have. If I type this-> I can then see all methods and members of a class. Type a few more letters, and see all of them with a certain prefix, like get or set. Saves on typing, checking names, and prevents typos.

This cannot work for unqualified names, in part because the set of candidates can be large, and in part because of argument dependent lookup. But when calling a method on a class, you don't want argument dependent lookup.

#20 Pink Horror   Members   -  Reputation: 1156

Like
0Likes
Like

Posted 27 August 2013 - 02:52 PM


The point is not that it causes ambiguity or strange behavior.

 

Your original point was that it causes side effects. Are side effects not ambiguity or strange behavior?






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS