Hacker News Reader

Comment by messe | Hacker News Reader

You have an off-by-one error. The strings are 3 bytes long.

I'd write something similar, more or less; Probably the following, not for optimization, but more as a matter of style:

    const char countries[][3] = {
        "A1",
        "A0",
       // [...]
    };

    // [...]

    int total_countries = (int) (sizeof(countries) / sizeof(countries[0]));
    return cc < total_countries ? countries[cc] : "UNKNOWN";

No need to hard code the length, and the cast is guaranteed to be within the bounds of an int on all platforms as long as you don't go over 2^16-1 countries.

View on HN

I think that's the best solution (the only potential optimization I can think of would be to add a padding byte after every country to force 32bit alignment which would make the offset computation faster on some architectures, although probably not on x86).

Stylistically-wise I think the best solution would be to write the array like:

    const char countries[][3] = {
        [0] = "A1",
        [1] = "A0",
    };

This way the codes are explicit when you read the code and it makes editing the array a little easier. Unfortunately gcc only warns if you set the same index twice with -Wextra, it remains silent with -Wall.

> This way the codes are explicit when you read the code

Good call. It might have caught the off-by-one mistake I made (the codes start from 1 and not 0 in the blog post). Maybe even switch to using an enum as our index instead of an int.

Well caught! I have two off by one errors :D seems they start from 1 instead of 0, and I didn't account for the null byte :D your solution is nicer and easier to read :)

This jives with the overall point of the article of keep it simple and readable. The semi fancy indexing caused 2 errors and the upside is negligible after the compiler gets done with it.

Reply on news.ycombinator.com