views:

47

answers:

4

In COM when I have a well-known interface that I can't change:

interface IWellKnownInterface {
     HRESULT DoStuff( IUnknown* );
};

and my implementation of IWellKnownInterface::DoStuff() can only work when the passed object implements some specific interface how do I handle this situation?

HRESULT CWellKnownInterfaceImpl::DoStuff( IUnknown* param )
{
    //this will QI for the specific interface        
    ATL::CComQIPtr<ISpecificInterface> object( param );
    if( object == 0 )
        //clearly the specifil interface is not supported
        return E_INVALIDARG;
    }
    // proceed with implementation
}

In case the specific interface is not supported which error code should I return? Is returning E_INVALIDARG appropriate?

A: 

Yes. At least, that is what I've learned to expect a microsoft API call to return in such a case.

Paul-Jan
+1  A: 

E_INVALIDARG is a fine choice, but the most important thing is to ensure that the precise conditions for each return code that you use are well documented.

Additionally, you could consider implementing ISupportErrorInfo and then returning rich error information via CreateErrorInfo and SetErrorInfo. This is especially useful in cases where you think callers may benefit from having a custom error message generated at the point of failure, with all of the relevant context contained therein. In your case, this might be to identify specifically which argument is invalid and which interface was unimplemented for it to be so. Even though such a message is unlikely to be of value to an end user, it could be invaluable to a developer if it shows up in a log file or the event viewer.

Phil Booth
A: 

I beg to differ with @nobugz. E_NOINTERFACE has the very specific meaning that your object does not support a certain interface that your client has just requested.

In general, you should only return E_NOINTERFACE from your own IUnknown::QueryInterface(). Returning E_NOINTERFACE from DoStuff() would surprise end users and tools alike. If I saw E_NOINTERFACE coming from anywhere other than QueryInterface(), my immediate thought would be "abstraction leak!".

At first sight, E_INVALIDARG looks like the best options available -- after all, you were passed an argument that doesn't work for you. But in my view there is a subtlety here, so I'm not sure if I can make a universal rule out of that.

In an ideal world, the error codes returned by a COM method are more about the Interface than about the object. Concretely, IWellKnownInterface::DoStuff() doesn't define (necessarily) that the object being passed in must implement ISpecificInterface, or it would have a different argument type. So, technically you're cheating potential users of your object by not quite fully implementing the semantics of your interface. Of course, in the real world, it's very hard to create a meaningful open system if you make that into an absolute rule and interfaces are that rigid.

So: if you can provide reduced functionality that still meets the interface semantics even if your argument doesn't implement ISpecificInterface, you should consider doing that.

Assuming that you must return in error: if this is a well-know, well documented (well designed?) interface, I would probably glance at the interface documentation first, to see if they give you any guidance. Lacking that, I would probably go ahead and use E_INVALIDARG.

@Phil Booth has some nice recommendations about how to provide more information to your user about the nature of the error.

Obviously, if you had the chance you would want to change the interface to take a ISpecificInterface* param instead.

Euro Micelli
Well, I disagree. Tell the client what went wrong, don't tell him what you think went wrong. There's only one shot at this, there are no nested error codes. It is a similar kind of mistake as catch (Exception) { log("File not found"); }. I suppose I have to down-vote you too now.
Hans Passant
@nobugs: I agree 100% on telling the client what's wrong, but I don't think that changing the meaning of `E_NOINTERFACE` is correct (e.g, http://blogs.msdn.com/oldnewthing/archive/2006/12/08/1239911.aspx , although it's clearly NOT the same situation by any stretch; that was much worse). Even if the client programmer understood that E_NOINTERFACE applied to the argument, that wouldn't be much better because it still wouldn't indicate *which* interface was needed.
Euro Micelli
A: 

Here's another possibilty you might or might not like better.

In Visual Basic 6, there are many objects with methods that take a VARIANT argument and look for several alternative interfaces they can use (e.g. the DataSource member of data-bound controls). That looks a lot like your situation. Those objects return DISP_E_TYPEMISMATCH, which is a well understood return value by most people.

On the other hand, I totally understand if you don't want to return a DISP_xxxx error code from non-IDISPATCH based interfaces. There is a strong argument for saying that DISP_xxxx error codes are intened for VARIANT parameters only, and a method that takes an IUnknown* should not be returning error them.

I'm on the fence on this one.

Euro Micelli