6
\$\begingroup\$

I attempting to merge 3 functions into one, as they contain for the most part the same code.

Here are the original functions:

extern "C" PyObject *method_noargs_call_handler( PyObject *_self_and_name_tuple, PyObject * )
{
    try
    {
        Tuple self_and_name_tuple( _self_and_name_tuple );

        PyObject *self_in_cobject = self_and_name_tuple[0].ptr();
        void *self_as_void = PyCapsule_GetPointer( self_in_cobject, NULL );
        if( self_as_void == NULL )
            return NULL;

        ExtensionModuleBase *self = static_cast<ExtensionModuleBase *>( self_as_void );

        Object result( self->invoke_method_noargs( PyCapsule_GetPointer( self_and_name_tuple[1].ptr(), NULL ) ) );

        return new_reference_to( result.ptr() );
    }
    catch( Exception & )
    {
        return 0;
    }
}

extern "C" PyObject *method_varargs_call_handler( PyObject *_self_and_name_tuple, PyObject *_args )
{
    try
    {
        Tuple self_and_name_tuple( _self_and_name_tuple );

        PyObject *self_in_cobject = self_and_name_tuple[0].ptr();
        void *self_as_void = PyCapsule_GetPointer( self_in_cobject, NULL );
        if( self_as_void == NULL )
            return NULL;

        ExtensionModuleBase *self = static_cast<ExtensionModuleBase *>( self_as_void );

        Tuple args( _args );
        Object result
                (
                self->invoke_method_varargs
                    (
                    PyCapsule_GetPointer( self_and_name_tuple[1].ptr(), NULL ),
                    args
                    )
                );

        return new_reference_to( result.ptr() );
    }
    catch( Exception & )
    {
        return 0;
    }
}

extern "C" PyObject *method_keyword_call_handler( PyObject *_self_and_name_tuple, PyObject *_args, PyObject *_keywords )
{
    try
    {
        Tuple self_and_name_tuple( _self_and_name_tuple );

        PyObject *self_in_cobject = self_and_name_tuple[0].ptr();
        void *self_as_void = PyCapsule_GetPointer( self_in_cobject, NULL );
        if( self_as_void == NULL )
            return NULL;

        ExtensionModuleBase *self = static_cast<ExtensionModuleBase *>( self_as_void );

        Tuple args( _args );

//        if( _keywords == NULL )
//        {
//            Dict keywords;    // pass an empty dict

            Object result
                (
                self->invoke_method_keyword
                    (
                    PyCapsule_GetPointer( self_and_name_tuple[1].ptr(), NULL ),
                    args,
                     _keywords ? Dict{ _keywords } : Dict { } // was: keywords
                    )
                );

            return new_reference_to( result.ptr() );
//        }
//        else
//        {
//            Dict keywords( _keywords ); // make dict
//
//            Object result
//                    (
//                    self->invoke_method_keyword
//                        (
//                        PyCapsule_GetPointer( self_and_name_tuple[1].ptr(), NULL ),
//                        args,
//                        keywords
//                        )
//                    );
//
//            return new_reference_to( result.ptr() );
//        }
    }
    catch( Exception & )
    {
        return 0;
    }
}

You can see that already I have tidied up the third one.

And here is my merge:

extern "C" PyObject *omni_call_handler(
                                       uint16_t flag,
                                       PyObject* _self_and_name_tuple,
                                       PyObject* _args,
                                       PyObject* _keywords
                                       )
{
    try
    {
        Tuple self_and_name_tuple( _self_and_name_tuple );

        PyObject* self_in_c_object = self_and_name_tuple[0].ptr();
        PyObject* name_in_c_object = self_and_name_tuple[1].ptr();

        void* self_as_void = PyCapsule_GetPointer( self_in_c_object, nullptr );
        void* name_as_void = PyCapsule_GetPointer( name_in_c_object, nullptr );

        if( self_as_void == nullptr  ||  name_as_void == nullptr )
            return nullptr;

        ExtensionModuleBase* self = static_cast<ExtensionModuleBase *>( self_as_void );

        switch( flag )
        {
            case METH_NOARGS:
            {
                Object result(
                              self->invoke_method_noargs(
                                                         name_as_void ) );
                return new_reference_to( result.ptr() );
            }

            case METH_VARARGS:
            {
                Tuple args( _args );
                Object result(
                              self->invoke_method_varargs(
                                                          name_as_void,
                                                          args
                                                          ) );
                return new_reference_to( result.ptr() );
            }

            case METH_KEYWORDS:
            {
                Tuple args( _args );
                Object result(
                              self->invoke_method_keyword(
                                                          name_as_void,
                                                          args,
                                                          _keywords ? Dict{ _keywords } : Dict { } // was: keywords
                                                          ) );
                return new_reference_to( result.ptr() );
            }
        }

        return 0;
    }

    catch( Exception & )
    {
        return 0;
    }
}

The only thing I'm not too happy about is having an identical return statement inside each switch case.

Is there a cleaner way to handle this?

\$\endgroup\$
2
\$\begingroup\$

I can't comment on the Python specifics, but regarding you question:

Instead of returning from each case, assign the reference to a variable, and return the value of the variable once, after the switch.

PyObject *result_ref;

switch ( flag ) {
    case 1:
    result_ref = new_reference_to( get_result_1().ptr() );
    break;

    case 2:
    result_ref = new_reference_to( get_result_2().ptr() );
    break;

    default:
    result_ref = nullptr;
}

return result_ref;    // Still return nullptr if flag is invalid

To further reduce code duplication, you could set an Object variable in the switch instead. Then do the new_reference_to( result.ptr() ) if the flag was valid, after the switch.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.