Function pointers and switch statements

I feel like I can make my switch statements more elegant with a function pointer, i.e. I'd like to set the digestLength and a pointer to a function in the same switch statement instead of setting the length, declaring the result array, then calling the function.

- (NSString *)hashWithDigestType:(NSStringDigestType)type {
    const char *s = [self UTF8String];
    int digestLength;

    switch (type) {
        case NSStringDigestTypeMD5:
            digestLength = CC_MD5_DIGEST_LENGTH;
            break;
        case NSStringDigestTypeSHA1:
            digestLength = CC_SHA1_DIGEST_LENGTH;
            break;
        case NSStringDigestTypeSHA512:
            digestLength = CC_SHA512_DIGEST_LENGTH;
            break;
    }

    unsigned char result[digestLength]; 

    switch (type) {
        case NSStringDigestTypeMD5:
            CC_MD5(s, strlen(s), result);
            break;
        case NSStringDigestTypeSHA1:
            CC_SHA1(s, strlen(s), result);
            break;
        case NSStringDigestTypeSHA512:
            CC_SHA512(s, strlen(s), result);
            break;
    }

    NSMutableString *digest = [NSMutableString stringWithCapacity:(digestLength * 2)];
    for (NSUInteger i = 0; i < digestLength; i++)
        [digest appendFormat:@"%02x",result[i]];

    return [NSString stringWithString:digest];
}

I can't write the code in Objective-C, but in C, you could create yourself a structure type to get the information in one move. I don't know whether the NSStringDigestTypeXXX values are conveniently numbered compactly from 0 or 1, so I'm taking the pessimistic view that they are not. You can simplify the code below if they are compact and small.

struct Digestifier  // Declaration in a header (probably)
{
    int      hashtype;
    void   (*hash)(const char *source, size_t length, char *result);
    size_t   hashlen;
};
static const struct Digestifier digests[] =
{
    { NSStringDigestTypeSHA1,   CC_SHA1,   CC_SHA1_DIGEST_LENGTH   },
    { NSStringDigestTypeMD5,    CC_MD5,    CC_MD5_DIGEST_LENGTH    },
    { NSStringDigestTypeSHA512, CC_SHA512, CC_SHA512_DIGEST_LENGTH },
};
{ enum NUM_DIGESTS = sizeof(digests) / sizeof(digests[0]) };

You can then write a lookup function for this:

const struct Digestifier *digest_lookup(int hashtype)
{
    for (i = 0; i < NUM_DIGESTS; i++)
    {
        if (digests[i].hashtype == hashtype)
            return &digests[i];
    }
    assert(i != NUM_DIGESTS);  // Or other error handling!
    return 0;
}

And in your function:

- (NSString *)hashWithDigestType:(NSStringDigestType)type {
    const char *s = [self UTF8String];
    const struct Digestifier *digest = digest_lookup(type);

    // Error check digest if digest_lookup() does not do it for you!
    unsigned char result[digest->hashlen];
    digest->hash(s, strlen(s), result);

    NSMutableString *digest = [NSMutableString stringWithCapacity:(digest->hashlen * 2)];
    for (NSUInteger i = 0; i < digestLength; i++)
        [digest appendFormat:@"%02x",result[i]];

    return [NSString stringWithString:digest];
}

Note that you could also write the hash function invocation as:

(*digest->hash)(s, strlen(s), result);

To some of us old-school (pre-standard) C programmers, that might perhaps be clearer.

Also, if Objective-C supports the C99 designated initializer notation, you could make the initializer for the digests[] array more robust (and render the hashtype member superfluous except for a cross-check):

static const struct Digestifier digests[] =
{
    [NSStringDigestTypeSHA1]   =
           { NSStringDigestTypeSHA1,   CC_SHA1,   CC_SHA1_DIGEST_LENGTH   },
    [NSStringDigestTypeMD5]    =
           { NSStringDigestTypeMD5,    CC_MD5,    CC_MD5_DIGEST_LENGTH    },
    [NSStringDigestTypeSHA512] =
           { NSStringDigestTypeSHA512, CC_SHA512, CC_SHA512_DIGEST_LENGTH },
};

This initializer correctly places the three rows in the array regardless of which member of the enumeration is mapped to 0, 1, 2.

With the additional information that the NSStringDigestTypeXXX values are 0, 1, 2, you can simplify the digest_lookup() function by:

• Ensuring that the rows in the digests array are in the correct (0, 1, 2) sequence.

• Changing from a search loop to a direct array lookup.

• Probably asserting that the value is under control.

For the purposes of the code below, I'm assuming that NSStringDigestTypeMD5 is 0 and NSStringDigestTypeSHA512 is 2, but the ordering of the names is arbitrary; just choose the equivalent of 0 for the first name in the assert and 2 for the second.

const struct Digestifier *digest_lookup(NSStringDigestType hashtype)
{
    assert(hashtype >= NSStringDigestTypeMD5 &&
           hashtype <= NSStringDigestTypeSHA512);
    assert(digest[hashtype].hashtype == hashtype);
    return &digests[hashtype];
}

The first assertion ensures that the value is in range. The second assertion ensures that the table is properly sequenced and you are getting back the entry your expect.

StackExchange Code Review Q#858, answer score: 4