1

The code below is a solution to the problem shown in the bullet points. Is the way the -1 return case is handled faulty?

  • An input string is guaranteed to be made up of only lower case English letters.
  • Return -1 if there are no unique characters (characters that only appear once) in the string.
  • Otherwise, return the index [from the input string] of the first unique character in the string.
int64_t FirstUniqueCharInString(const std::string& s)
{
    std::array<bool, 26U> seen{};
    std::array<std::string::size_type, 26U> pos{};
    pos.fill(std::string::npos);

    for (auto i = static_cast<std::size_t>(0); i < s.size(); ++i)
    {
        auto array_index_for_current_char = static_cast<std::size_t>(s[i] - 'a');
        pos.at(array_index_for_current_char) = seen.at(array_index_for_current_char) ?
                                               std::string::npos : i;
        seen.at(array_index_for_current_char) = true;
    }

    // static casting result and std::string::npos to int64_t to match return type of function which
    // is int64_t to allow for negative values (we have to be able to return -1 when there are no unique chars).
    int64_t result = static_cast<int64_t>(*std::min_element(pos.begin(), pos.end()));
    return result == static_cast<int64_t>(std::string::npos) ? -1 : result;
}


8
  • int64_t is a signed integer type. So it allows for negative numbers. Compare that to uint64_t. Commented Sep 5, 2022 at 4:22
  • You don’t need to cast the result then cast npos. Just check if the result is npos then return what you want.
    – Taekahn
    Commented Sep 5, 2022 at 4:25
  • @Taekahn Yup, you don't need to cast the result. I only did it to be explicit. Without the cast there is an implicit conversion from the "world" of unsigned, which was needed for result (or, if you prefer, elements of the pos array) earlier in the solution since those elements were of size type.... to the "world" of signed, which is needed for the return type so that the -1 case can be returned when applicable (i.e. when there is no unique char).
    – tarstevs
    Commented Sep 5, 2022 at 4:43
  • No, that’s not what I mean. No conversion needs to take place. Implicit or explicit. You’re taking an unsigned number, converting it to signed. Then taking another unsigned number, and converting it to signed. Then checking to see if the two numbers are the same. You can literally just check them as unsigned numbers. But the more idiomatic way, imo, would be to simply check of the result of min_element == pos.end()
    – Taekahn
    Commented Sep 5, 2022 at 5:07
  • @Taekahn What do you propose for the return type of the function?
    – tarstevs
    Commented Sep 5, 2022 at 5:23

1 Answer 1

0

This code still has the problem of casting npos to int64_t, which is not guaranteed to return -1.

The last two lines could be made correct:

auto element = *std::min_element(pos.begin(), pos.end());
if ( element == std::string::npos )
     return -1;
return element;
1
  • Just to clarify, in the last line of code in the OP here, casting npos to int64_t is not serving the purpose of generating a -1. If I'm reading your comment correctly I believe it says that casting npos to int64_t is not going to work for generating a -1. To say what I'm trying to say differently, in that last line, two variables holding the int64_t "version" of npos are being compared using ==, and based on the outcome, the rvalue -1 is returned or the contents of result is returned.
    – tarstevs
    Commented Sep 5, 2022 at 19:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.