3

I am extensively using std::strchr() in my code, but recently i started thinking about making my code more readable and modern. I wish there was function similar to std::any_of/std::string::find_first_of which is taking single character instead of containers. So i am questioning myself how to "update" my code to C++17. 

while (std::strchr("abcd", input) == nullptr) { //how to get rid of this C function?
        //do smth
}

Any ideas?

Thanks, have a nice day!

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
JoJo
  • 43
  • 1
  • 4

5 Answers5

3

You can use std::find with a string, or std::strings very own std::string::find.

SergeyA
  • 61,605
  • 5
  • 78
  • 137
3

There is no sense to update your code because the string literal has a type of a character array.

It would be a bad idea to create an intermediate object of for example std::string to perform such a simple task.

With c-strings declared like arrays use C string functions. C string functions are optimized and sometimes are performed by using just a pair of machine instructions.

With other containers use their member functions or standard algorithms.

Compare for example two approaches

const char *s = "abcd";

const char *p = strchr( s, c );

if ( p )
{
    //...
}

Or even like

const char *s = "abcd";

if ( const char *p = strchr( s, c ) )
{
    //...
}

and 

const char *s = "abcd";
size_t n = std::strlen( s );

auto it = std::find( s, s + n, c );

if ( it != s + n )
{
    //...
}

Or less readable in C++ 17

const char *s = "abcd";
size_t n = std::strlen( s );

if ( auto it = std::find( s, s + n, c ); it != s + n )
{
    //...
}

It is evident that the first approach is more efficient.

On the other hand, if you have a general function that should accept c-strings and/or objects of the type std::string then if the function does not change them then use std::string_view as a function parameter.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

If you can store the C string in an array, you can use std::find like so:

constexpr char charset[] = "abcd";
while (std::find(std::begin(charset), std::end(charset), input) 
       == std::end(charset)) 
{...}
Felix Dombek
  • 13,664
  • 17
  • 79
  • 131
  • In general this approach is not suitable because the size of the array can be greater than the size of the stored string. – Vlad from Moscow Sep 20 '19 at 16:10
  • @vlad You mean if `charset[50] = "abcd"`? True but not sure if that case falls under things you need to take into account ... I could also say: in the general case (for huge arrays) this whole approach is bad, because it has linear runtime, better put your charset in a hashmap first and the lookup in the loop will be much faster. – Felix Dombek Sep 20 '19 at 16:17
  • Apart from this situation the program can deal with pointers to first characters of c-strings. In this case your approach is just invalid. – Vlad from Moscow Sep 20 '19 at 16:20
  • @vlad That's exactly why I included my first sentence, and also the first line of code. If you have your charsets as literals in the program, you can easily pull them out into arrays. Otherwise, it's not so simple. – Felix Dombek Sep 20 '19 at 16:22
1

It looks like the modern c++ alternative to strchr is std::char_traits<char>::find. It's also constexpr!

while (std::char_traits<char>::find("abcd", input) == nullptr) { //how to get rid of this C function?
    //do smth
}

However, I'm not sure that's any more readable than what you already have. If your goal is purely readability, then there's nothing wrong with wrapping standard library functions with thin inline wrappers.

inline bool DoesStringContainSubstring(const char* str, const char* subStr) { return std::strchr(str, subStr) != nullptr; }
Dan Forever
  • 381
  • 2
  • 12
0

You can use std::string's find_first_of. Example:

std::string string{"abcdefglrkemf..."};

while (string.find_first_of("def") != std::string::npos)
    //...

But if you don't want to create a new std::string object, strstr is the way to go. But note that this has a performance hit.

Arush Agarampur
  • 1,340
  • 7
  • 20