CODEWITHSUNDEEP
c++
Baz
Baz

Reputation: 13125

Working with interfaces

I have functions of the following form in the code I'm refactoring:

A f()
{
    if(existing)
        return A();
    else
        return A(handle);
}

The Safe Bool Idiom is later used to test if A is associated with a handle or not, i.e. if we should call the class methods for this object which require internally a valid handle for execution. A's methods are const.

I'd like to return an interface, IA, here instead. Must I therefore return a pointer? If so I will go with boost shared pointers. I can test if the pointer is pointing to something or not.

Is there any way for me to work with references here instead? Would you recommend such an approach or do you think that boost::shared_ptrs is the way to go?

UPDATE

A is derived from IA.

My compiler is gcc version 4.4.3.

My biggest problem with this code is that A is used to interact with an external C API. Therefore I wish to mock it away using the IA interface as base for my Mock of A and its implementation, A. Then outside of the method f() above, which I see as a factory, I will only work with IA pointers. Dependency Injection in other words.

So A is basically a handle and an interface to a set of C API functions which require a handle. I can have several objects of type A where the interface is the same but the handle is different.

Upvotes: 2

Views: 161

Answers (5)

Luchian Grigore
Luchian Grigore

Reputation: 258558

I'd go with pointers too, but you can also work with references:

A& f()
{
    if(existing)
    {
        static A a;
        return a;
    }
    else
    {
        static A a(handle);
        return a;
    }
}

You are fully aware of the implications though, right? i.e. you can't re-assign the reference and modifying it means modifying the local static variable.

Upvotes: 2

BЈовић
BЈовић

Reputation: 64213

I'd return a std::unique_ptr< AI > object :

std::unique_ptr< AI > f()
{
    if(existing)
        return std::unique_ptr< AI >( new A() );
    else
        return std::unique_ptr< AI >( new A(handle) );
}

In the above case, the slicing doesn't happen, and the compiler will move* the object.

* I assumed you are using the c++11.

Since you are not using c++11, the simplest is to use boost::shared_ptrs :

boost::shared_ptrs< AI > f()
{
    if(existing)
        return boost::shared_ptrs< AI >( new A() );
    else
        return boost::shared_ptrs< AI >( new A(handle) );
}

In such case, you do not have to take care whether and when the created object gets destructed. The boost::shared_ptrs will take care of that.

Upvotes: 4

Agent_L
Agent_L

Reputation: 5411

If you can understand pointers then work with pointers. If you can't, then stay with what you've got. Looks like the guy before you did his best to avoid pointers. As betabandido said, the compiler will make best out of it, even if it seems slow - on paper.

Interface is a design pattern to take fullest advantage of pointers. It doesn't make very much sense without pointers and casting.

To test if the pointer is pointing at something or not, there is NULL value. No need to roll out cannons if you're shooting flies.

Explain why you're not comfortable with the code as it is now. Maybe the problem isn't really severe.

Upvotes: 1

betabandido
betabandido

Reputation: 19704

From your code snippet it seems that you are constructing A objects in function f. In such a case, returning the object by value is probably the best thing you can do. The compiler will use return value optimization (RVO) and optimize all the copies away.

Check this article by Dave Abrahams on passing and returning objects by value.

Be aware that this solution will not work if you are returning a base class of A, due to the slicing problem. If returning an A object is fine, then this is probably the best solution.

Upvotes: 1

Eric Z
Eric Z

Reputation: 14505

Returning a pointer is one way to go.

To me, another simple and native way is to add a return value to the method's signature.

int f(A & a)
{
    if(existing)
    {
        return ERROR_ALREADY_EXISTS;
    }
    else
    {
        A temp(handle);
        a = temp;
        return SUCCEEDED;
    }
}

Upvotes: 0

Related Questions