How can i optimize this Codewars c++ code?

Question

i was doing a Codewars training in c++ and my code is working good but it says that it is not fast enough to pass all the tests. In this training, i need to check if a portion of str1 characters can be rearranged to match str2 (all passed as parameters and being const). Pretty simple subject but how can i optimize it ? Here's my code :

bool scramble(const std::string& s1, const std::string& s2){
   int j = 0;
   int i = 0;
   size_t good_count = 0;
   char tmp[s1.size()];

   std::strcpy(tmp, s1.c_str());
   while (tmp[i]) {
        if (tmp[i] == s2[j]) {
            std::memmove(&tmp[i], &tmp[i + 1], strlen(tmp) - i);
            j++;
            good_count++;
            i = 0;
        } else
            i++;
      }
     return good_count == s2.size() ? true : false;
}

I thought about the memmove function, is it too slow ? I tried with std::remove and str.erase but i get the same results. Thank you for your help !

I guess sorting the characters of each string would be usefull — Damien, May 16 '20 at 08:11
For optimizing working code, please post on [codereview.se]. — Filburt, May 16 '20 at 08:11
Yes but sorting each characters isn't taking too much time to process ? — Silent observer, May 16 '20 at 08:16
Thanks for sharing Code Review tip, i will use this in the future. — Silent observer, May 16 '20 at 08:17
You could pass s1 by non const copy to avoid having tmp, this way the original value will not be changed, yet in the scope of your function you will have a temporary s1. You can remove the ternary at the end, operator== returns a bool already — Benjam, May 16 '20 at 08:30
Sorting is performed in O(n logn). See `std::sort`, can be used on `std::string` — Damien, May 16 '20 at 08:37
Comparison sort is O(n lg N) but count/radex sort is only O(N), it avoids all the comparisons and therefore the branch mispredicts. — Surt, May 16 '20 at 08:48

score 1 · Accepted Answer · answered May 16 '20 at 08:20

Your current metode is to actually do a rearrangement, with a lot of memory movement. But actually you could just count the amount of each letter in S2, then run through S1 until you have accumulated the same number of letters.

Achtung totally untested code.

// assuming all is lowercase
int matches = S2.length();
std::array<int, 256> chars { 0 };
for (char alfa : S2) {
  chars[alfa]++;
}
for (char alfa : S1) {
  if (chars[alfa]-->0) {
    if (!--matches)
      return true;
  }
}

Dankeschön ! I understand now that i could just count instead of trying to remove char by char. I'll try this method right now ! — Silent observer, May 16 '20 at 08:31

How can i optimize this Codewars c++ code?

1 Answers1