HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Template function specialization with code duplication

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
templatewithduplicationspecializationfunctioncode

Problem

I'm working on a libcurl binding for NodeJS, and to get info from a curl handler, using curl_easy_getinfo, I've created a template function, the issue, is that the result of the call can be a null pointer.

To make sure this was correctly addressed, I've created a template specialization that handle the edge case:

template
v8::Handle Curl::GetInfoTmpl( const Curl &obj, int infoId )
{
    v8::HandleScope scope;

    ResultType result;

    CURLINFO info = (CURLINFO) infoId;
    CURLcode code = curl_easy_getinfo( obj.curl, info, &result );

    if ( code != CURLE_OK )
        return Curl::Raise( "curl_easy_getinfo failed!", curl_easy_strerror( code ) );

    return scope.Close( v8MappingType::New( result ) );
}

//The null pointer is only possible when the value returned is a c string
template<> //workaround for null pointer, aka, hack. 
v8::Handle Curl::GetInfoTmpl( const Curl &obj, int infoId )
{
    //duplicated code
    v8::HandleScope scope;

    char *result;

    CURLINFO info = (CURLINFO) infoId;
    CURLcode code = curl_easy_getinfo( obj.curl, info, &result );

    if ( !result ) { //null pointer
        return v8::String::New( "" );
    }

    if ( code != CURLE_OK )
        return Curl::Raise( "curl_easy_getinfo failed!", curl_easy_strerror( code ) );

    return scope.Close( v8::String::New( result ) );
}


Is this the correct way to handle that, or are there better ways to do so?

The full code is on GitHub.

Solution

The check is better factored into a specialization on a traits class. This allows you to have one implementation of the overall logic flow, and just specialize the part that changes. I haven't fully researched the implications of this approach, but I expect that due to the always true/false nature of the check (for most given types), that it will be easily optimized away.

It would look something like this:

// traits class to determine whether to do the check
template  struct ResultCanBeNull : std::false_type {};
template <> struct ResultCanBeNull : std::true_type {};

template
v8::Handle Curl::GetInfoTmpl( const Curl &obj, int infoId )
{
    v8::HandleScope scope;

    ResultType result;

    CURLINFO info = (CURLINFO) infoId;
    CURLcode code = curl_easy_getinfo( obj.curl, info, &result );

    // use the traits class to filter in or out the null pointer check
    if ( ResultCanBeNull::value && !result ) { //null pointer
        return v8::String::New( "" );
    }

    if ( code != CURLE_OK )
        return Curl::Raise( "curl_easy_getinfo failed!", curl_easy_strerror( code ) );

    return scope.Close( v8MappingType::New( result ) );
}


You could also implement the traits support as a method instead, in case it's not valid to call !result on instances of other types that are reasonable for ResultType.

template 
struct ResultTraits {
    bool IsNull(const ResultType&) { return false; }
};
template<> bool ResultTraits::IsNull(char const*& pchar) {
    return !pchar;
};

: : :

    if (ResultTraits::IsNull(result)) {
        return v8::String::New( "" );
    }

Code Snippets

// traits class to determine whether to do the check
template <typename> struct ResultCanBeNull : std::false_type {};
template <> struct ResultCanBeNull<char*> : std::true_type {};

template<typename ResultType, typename v8MappingType>
v8::Handle<v8::Value> Curl::GetInfoTmpl( const Curl &obj, int infoId )
{
    v8::HandleScope scope;

    ResultType result;

    CURLINFO info = (CURLINFO) infoId;
    CURLcode code = curl_easy_getinfo( obj.curl, info, &result );

    // use the traits class to filter in or out the null pointer check
    if ( ResultCanBeNull<ResultType>::value && !result ) { //null pointer
        return v8::String::New( "" );
    }

    if ( code != CURLE_OK )
        return Curl::Raise( "curl_easy_getinfo failed!", curl_easy_strerror( code ) );

    return scope.Close( v8MappingType::New( result ) );
}
template <typename ResultType>
struct ResultTraits {
    bool IsNull(const ResultType&) { return false; }
};
template<> bool ResultTraits<char const*&>::IsNull(char const*& pchar) {
    return !pchar;
};

: : :

    if (ResultTraits<ResultType>::IsNull(result)) {
        return v8::String::New( "" );
    }

Context

StackExchange Code Review Q#50943, answer score: 3

Revisions (0)

No revisions yet.