Prev: Arguement against NULLing a pointer after delete?
Next: Virtual inheritance and typedef
From: Greg Herlihy on 12 Nov 2009 09:11
On Nov 11, 2:50 pm, "Martin B." <0xCDCDC...(a)gmx.at> wrote:

> if(shared_ptr<MyWhatever> pX = shared_ptr<MyWhatever>(get_whatever())) {
>
> }
>
> This is ugly (and presumably could also add some runtime overhead).
>
> Is there a better way?

How about:

typedef shared_ptr<MyWhatever> MyWhateverPtr;

MyWhateverPtr pX(get_whatever());

if (pX)
{
// ...
}

Greg



--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Martin B. on 13 Nov 2009 00:49
Joshua Maurice wrote:
> On Nov 12, 10:30 am, Jeff Flinn <TriumphSprint2...(a)hotmail.com> wrote:
>> Martin B. wrote:
>>> Hi all.
>>> // With a pointer I can do:
>>> if(MyWhatever* pX = get_whatever()) {
>>> }
>>> However, for this to work with a shared pointer (shared_ptr or
>>> scoped_ptr from boost) I have to do:
>>> if(shared_ptr<MyWhatever> pX = shared_ptr<MyWhatever>(get_whatever())) {
>>> }
>>> This is ugly (and presumably could also add some runtime overhead).
>>> Is there a better way?
>> typedef shared_ptr<MyWhatever> MyWhateverPtr; // ;-)
>>
>> if(MyWhateverPtr pX = MyWhateverPtr(get_whatever()))
>> {
>> ...
>> }
>>

*Sigh* :)
( I wasn't concerned about the <>, but about the repetion of the type
and possible runtime overhead. )

>> or even less redundant and more safe: make get_whatever return a
>> MyWhateverPtr. If you can't modify the get_whatever() wrap it in another
>> function that returns a MyWhateverPtr.
>
> Adding a one line helper function to use in one case seems very much
> like overkill and obfuscation. What's wrong with the following?
>
> MyWhateverPtr pX(get_whatever());
> if(pX.get())
> {
> //...
> }
>

*Sigh* :)
( Object does not have minimal scope - that's what's wrong. )

> If you really want to restrict the scope of pX, then add braces:
>
> {
> MyWhateverPtr pX(get_whatever());
> if(pX.get())
> {
> //...
> }
> }
>
> (...)

*Sigh* :)
( Except for rare cases, I consider this more clutter that it's worse. )


Let me rephrase:

C++ as of today, offers a very neat syntax feature for initializing
pointers:
if(ptr_t* p = fn_returns_pointer()) { ... }

However, for such functions as need to free their return pointers, it
would be prudent to use a smart pointer. (the appropriate shared_ptr
type could invoke whatever free/delete/dll_release function is necessary)

Unfortunately, it is not possible to use this convenient syntax with
smart pointer objects, since a properly written smart pointer must not
allow implicit assignment from the underlying pointer-type.
So, unfortunately, with smart pointers we are stuck with the following
syntax:
MySharedPtr xhandle( fn_returns_pointer_needing_release() );
if(xhandle) { ... }

-> The lifetime of xhandle is unneccesarily long.
-> 2 lines instead of one.

C++0x may improve this by:
if(auto xhandle = MySharedPtr( fn_returns_pointer_needing_release() )) {
.... }
but I still consider this less clear than with a raw pointer.

br,
Martin

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Jeff Flinn on 13 Nov 2009 03:41
Joshua Maurice wrote:
> On Nov 12, 10:30 am, Jeff Flinn <TriumphSprint2...(a)hotmail.com> wrote:
>> Martin B. wrote:
>>> Hi all.
>>> // With a pointer I can do:
>>> if(MyWhatever* pX = get_whatever()) {
>>> }
>>> However, for this to work with a shared pointer (shared_ptr or
>>> scoped_ptr from boost) I have to do:
>>> if(shared_ptr<MyWhatever> pX = shared_ptr<MyWhatever>(get_whatever())) {
>>> }
>>> This is ugly (and presumably could also add some runtime overhead).
>>> Is there a better way?
>> typedef shared_ptr<MyWhatever> MyWhateverPtr; // ;-)
>>
>> if(MyWhateverPtr pX = MyWhateverPtr(get_whatever()))
>> {
>> ...
>> }
>>
>> or even less redundant and more safe: make get_whatever return a
>> MyWhateverPtr. If you can't modify the get_whatever() wrap it in another
>> function that returns a MyWhateverPtr.
>
> Adding a one line helper function to use in one case seems very much
> like overkill and obfuscation. What's wrong with the following?
>
> MyWhateverPtr pX(get_whatever());
> if(pX.get())
> {
> //...
> }
>
> If you really want to restrict the scope of pX, then add braces:
>
> {
> MyWhateverPtr pX(get_whatever());
> if(pX.get())
> {
> //...
> }
> }
>
> This would be much quicker and easier for me to read then look at the
> code and ponder what this function does, only to see it's a near
> trivial one liner just to play to your particular liking for putting
> declarations in if statements. just my humble opinion.

Yes the latter is comparable, but the .get() is unneeded, shared_ptr's
interface allows it to be evaluated within a boolean expression.

My point is that functions returning raw pointers should be replaced
with functions returning the appropriate safe pointer to begin with. If
they can't be replaced directly then wrap them with a function that does
so, and disallow the unsafe functions from being used.

Jeff


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Ulrich Eckhardt on 13 Nov 2009 16:27
Martin B. wrote:
> Ulrich Eckhardt wrote:
>> Martin B. wrote:
>>> However, for this to work with a shared pointer (shared_ptr or
>>> scoped_ptr from boost) I have to do:
>>> if(shared_ptr<MyWhatever> pX = shared_ptr<MyWhatever>(get_whatever())) {
>>>
>>> }
>>
>> Do you? What is the declaration of get_whatever()? Did you change that to
>> return a shared_ptr, too? In that case, you could safe yourself the
>> explicit conversion. If not, you could at least use this syntax:
>>
>> if(shared_ptr<T> p(get_whatever()))
>>
>> Which works if I remember correctly.
>>
>
> You remember wrongly. I get a compiler error if I try this.

Well, I still don't really know what get_whatever() returns. In any case,
here's what I tried:

// three functions...
int* f();
std::auto_ptr<int> g();
boost::shared_ptr<int> h();

// ...times three ways to check
if(boost::shared_ptr<int> x = f()); // 1
if(boost::shared_ptr<int> x = g()); // 1
if(boost::shared_ptr<int> x = h());
if(boost::shared_ptr<int> x(f())); // 2
if(boost::shared_ptr<int> x(g())); // 2
if(boost::shared_ptr<int> x(h())); // 2
if(boost::shared_ptr<int> x = boost::shared_ptr<int>(f()));
if(boost::shared_ptr<int> x = boost::shared_ptr<int>(g()));
if(boost::shared_ptr<int> x = boost::shared_ptr<int>(h()));

Result of this was:
1. No implicit conversion.
This one actually makes sense, you wouldn't want this to work.
2. Error "expected primary expression before 'x'".
Question to the audience: Why doesn't this work? It's not like this could be
misparsed as function declaration.

Anyway, simple remedy:

template<typename T>
boost::shared_ptr<T>
make_shared(T* t)
{ return boost::shared_ptr<T>(t); }

if(boost::shared_ptr<int> x = make_shared(f()));

This performs the conversion to a shared pointer explicitly (good thing) but
determines the type of the shared pointer from the argument, so you don't
have to repeat it.

HTH

Uli


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Martin B. on 14 Nov 2009 05:46
Ulrich Eckhardt wrote:
> Martin B. wrote:
>> Ulrich Eckhardt wrote:
>>> Martin B. wrote:
>>>> However, for this to work with a shared pointer (shared_ptr or
>>>> scoped_ptr from boost) I have to do:
>>>> if(shared_ptr<MyWhatever> pX = shared_ptr<MyWhatever>(get_whatever())) {
>>>>
>>>> }
>>> Do you? What is the declaration of get_whatever()? Did you change that to
>>> return a shared_ptr, too? In that case, you could safe yourself the
>>> explicit conversion. If not, you could at least use this syntax:
>>>
>>> if(shared_ptr<T> p(get_whatever()))
>>>
>>> Which works if I remember correctly.
>>>
>> You remember wrongly. I get a compiler error if I try this.
>
> Well, I still don't really know what get_whatever() returns. In any case,

Just a plain pointer.

> here's what I tried:
>
> // three functions...
> int* f();
> std::auto_ptr<int> g();
> boost::shared_ptr<int> h();
>(...)
> if(boost::shared_ptr<int> x(f())); // 2
>(...)
> 2. Error "expected primary expression before 'x'".
> Question to the audience: Why doesn't this work? It's not like this could be
> misparsed as function declaration.
>

*Shrug* The important and bad thing is it doesn't work. :-/

> Anyway, simple remedy:
>
> template<typename T>
> boost::shared_ptr<T>
> make_shared(T* t)
> { return boost::shared_ptr<T>(t); }
>
> if(boost::shared_ptr<int> x = make_shared(f()));
>
> This performs the conversion to a shared pointer explicitly (good thing) but
> determines the type of the shared pointer from the argument, so you don't
> have to repeat it.
>

Good thing'd be if I already could do:
if(auto x = make_shared( f() )) ... :-)

You know, I just realized that I'd probably need a custom make_shared
function anyway since in situations where you get a raw pointer back
it's 50/50 that you want to call delete on it.
For example: When dealing with C APIs, most of the time you'd like to
call free() / When getting the memory from a DLL chances are you'd need
to call a custom deleter that calls the release fuction from the library.

cheers.
Martin

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

First  |  Prev  | 
Pages: 1 2
Prev: Arguement against NULLing a pointer after delete?
Next: Virtual inheritance and typedef