CODEWITHSUNDEEP
c++coding-efficiency
Tryb Ghost
Tryb Ghost

Reputation: 439

What is the most efficient way to check if a string matches one of the possible inputs?

I'm asked to accept both a number and a unit, where a unit can be cm, m, in or ft.

For this I have a loop as such

cout << "Please put in a number and its unit.\n";
while (cin >> val >> unit) {
    if (val == '|') { break; }
    cout << "Please put in a number and its unit.\n";
}

My question is, what is the best way to check the unit string, when it comes to code efficiency as well as readability? Does it make more sense to put a large if statement

    if (unit != "cm" && unit != "m" && unit != "in" && unit != "ft") {
        cout << "Unit " << unit << " not accepted.\n";
    }

Or is it better to have a vector which I initialise with all the units, and then check if it matches any of the units.

    if (find(units.begin(), units.end(), unit) == units.end()) {
        cout << "Unit " << unit << " not accepted.\n";
    }

... where vector<string> units = {"cm", "m", "in", "ft"};

Or is there another way which is much better in terms of efficiency and readability?

I hope this is the right place to ask this question. I thought about code review but it doesn't seem to fit a small question like this.. or does it?

Upvotes: 1

Views: 109

Answers (3)

JeJo
JeJo

Reputation: 32847

Does it make more sense to put a large if statement?

The only four entries, I would say that you do not need to use std::vector(and the allocation cost comes up with it) and an algorithm to test the case. If the length of the if the condition is bothering you, pack the condition to a lambda function and call with the user input.

Proper naming of lambda would give much readability than checking the variable directly inside the if statement.

const auto isMatchingUnit = [](const std::string& unit) noexcept -> bool {
    return unit == "cm" || unit == "m" || unit == "in" || unit == "ft";
};

if (isMatchingUnit(unit)) {
    // do something
} else {
    // do something else
}

That being said, if the number of possible input s are large, you might wanna pack the possible inputs to std::arrayor std::vector and apply the standard algorithms as per choice:

If the collection to be checked is sorted and unique you can

  • std::binary_search which returns true, if item is found.
  • std::lower_bound by checking std::lower_bound(container.cbegin(), container.cend(), input) != container.cend(), for true case.

If the collection to be checked is not sorted

  • std::find by checking std::find(container.cbegin(), container.cend(), input) != container.cend(), for true case.
  • std::any_of with a proper predicate.

Upvotes: 1

Mike Dunlavey
Mike Dunlavey

Reputation: 40679

Your "if" statement is not only more efficient, but efficiency doesn't matter because no matter how you do it, the IO will take about a billion times longer.

It's also the most maintainable because if you format the "if" statement as one line per unit string, then adding, removing, or renaming a unit is only a 1-line edit, and you can easily comment them out or in if you change your mind.

The time to use a vector is if you don't know what the units are until run time.

Upvotes: 1

Michael Chourdakis
Michael Chourdakis

Reputation: 11168

The most efficient would be using std::lower_bound on a sorted array of strings, so it doesn't search all of them. If you use direct comparison or std::find, then all will be searched.

For a small vector, it might not be important (it might even be slower than a direct comparison), but for larger sets it will.

Upvotes: 0

Related Questions