Function pointers and switch statements

Question

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];
}

Answer 1

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:

1. Ensuring that the rows in the digests array are in the correct (0, 1, 2) sequence.
2. Changing from a search loop to a direct array lookup.
3. 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.

Answer 2

Here's some 2 cents.

1. You should never use identifiers beginning with NS, as this prefix is reserved for Apple only. In fact, recently Apple has begun discouraging the use of two- or three-letter prefixes entirely.

2. [I'm not sure about this approach, but] consider implementing each hash calculation as separate methods; and simply using selectors instead of function pointers:

   - (NSString *) hashWithDigestType:(StringDigestType) type
   {
       SEL digestMethods[] = {
           @selector(hashWithMD5),
           @selector(hashWithSHA1),
           @selector(hashWithSHA256)
       };
   
       return [self performSelector:digestMethods[type]];
   }
   

   Consider also validating the type variable and the possibility of using designated initialisers, so that you can directly map the enum values to the appropriate selectors.