I write a function that should convert a string to a number. I see two possible variants to write it:
int convert(const std::string input) {
if (input == "one") {
return 1;
} else if (input == "two") {
return 2;
}
// etc.
return 0;
}
Or
int convert(const std::string input) {
static const map<string, int> table = {
{"one", 1},
{"two", 2}
// etc.
}
const auto result = table.find(input);
if (result == table.end())
{
return 0;
}
return result->second;
}
What way is more effective/acceptable/readable?
The answer greatly depends on how many different strings you are going to support by this.
A few strings: go with if-else. The effort needed to understand the code later is little.
A lot of strings: Create a map. The effort understanding the code is small compared to the effort reading a huge if-else construct. Probably, you will have to extend this list often. Adding data requires less typing.
I am not sure how smart C++'s map is using strings as keys. In the worst case, both have the same performance. If the list gets really huge, you might think of creating a hash value of the strings and use this as a key. This might improve performance greatly. You will have to make sure that collisions don't happen though. (A good hash algorithm and 64 bit hash size should be sufficient.) It might be that modern map implementations do this already.
For small set of text, I'd use a simple lookup table:
struct LookupTable {
const char* text;
int value;
};
const LookupTable table[] = {
{ "one", 1 },
{ "two", 2 }
};
int convert(const char* text) {
if (!text) return 0;
for (int i=0; i<sizeof(table)/sizeof(LookupTable); i++) {
if (strcasecmp(text, table[i].text) == 0 ) {
return table[i].value;
}
}
return 0;
}
For large set of text, I would consider using std::unordered_map<std::string,int>
, and perhaps custom hash function (bkdr hash or elf hash is good on words).
EDIT: As David pointed out in comment, if you don't want the ugly sizeof
, use the modern for-loop:
int convert(const char* text) {
if (!text) return 0;
for (auto& entry: table) {
if (strcasecmp(text, entry.text) == 0 ) {
return entry.value;
}
}
return 0;
}
An if-else
(or a switch
, if available to you) are good for small cases, and you can also control the order of the tests in case the most common tests can cut out the search quickly, you can test them first.
In many cases, a switch
is far better than a list of if-else
s. Both easier to read and very possibly faster. Though switch
is not the best choice with string
.
You could however switch on an enum
rather than using strings; this is certainly the better approach, except for a map
.
A map
or std::unordered_map
is far better for large numbers of possibilities or when you need these possibilities updated at runtime.
For a small number of possible input values, I would prefer solution 1 which is straightforward and probably has the best performance.
If the list of values becomes too big, then what you really need is a converter between integers and written numbers, and that really is a different story (see the "Humanizer" library referenced in the comment by NathanOliver
What way is more effective/acceptable/readable?
The if
/else
solution is the most efficient if you only have a couple values, and is certainly quite straightforward especially for people not used to the Standard Library, however it quickly devolves into a mess.
Thus, as soon as you reach a 5 or more items, switch to using a container.
Caveat: unfortunately, std::string_view
, which would avoid a memory allocation, is still not standard; for simplicity I will thus use std::string
, though if memory allocation is an issue, std::string_view
or a custom CStr
class would be better.
There are 3 valid choices:
std::map<std::string, int>
and std::unordered_map<std::string, int>
are the most intuitive choices, it's unclear which would be fasterstd::vector<std::pair<std::string, int>>
(sorted) will always be more efficient than std::map<std::string, int>
Thus, if efficiency is a concern:
int convert(std::string const& name) {
static std::vector<std::pair<std::string, int>> const Table = []() {
std::vector<std::pair<std::string, int>> result = {
{ "one", 1 },
{ "two", 2 },
{ "three", 3 },
{ "four", 4 }
};
std::sort(result.begin(), result.end());
return result;
}();
auto const it =
std::lower_bound(Table.begin(), Table.end(), std::make_pair(name, 0));
if (it != Table.end() and it->first == name) {
return it->second;
}
return 0;
}
A sorted array is, after all, the most efficient way to perform a binary search, because of a better cache behavior. It should also outperform std::unordered_map
on smallish inputs for the same reasons.
Of course, it is slightly less readable.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With