eb80
eb80

Reputation: 4738

Custom Implementation of strcmp in C++ not Working

I am scratching my head... why is the return statement inside strcmp_iter never being called?

When I run this function, the output is simply to count from 0 to 6 and then terminate... no return statement. Very frustrating. Interestingly, if I change myString2 to "abcdefG" then everything works fine... very odd.

int strcmp_iter(string s1, string s2) {
  int i = 0;
  for (; ((s1.at(i) == s2.at(i)) && (i <= s1.length())); i++) {
    cout << i << endl;
  }
  return s1.at(i) - s2.at(i);
}


int main() {
  string myString1 = "abcdefg";
  string myString2 = "abcdefg";

  int count_iter = strcmp_iter(myString1, myString2);
  cout << "Iter: " << count_iter << endl;

  return 0;
}

Upvotes: 0

Views: 353

Answers (3)

wallyk
wallyk

Reputation: 57794

Whenever I see this construction, it makes me cringe:

 for (; ((s1.at(i) == s2.at(i)) && (i <= s1.length())); i++)

C-like languages always do short circuit evaluation and from left to right. So the condition for termination—and validating that the comparison makes sense to do—should precede the comparison:

 int n = s1.length();
 if (s2.length() < n)
       n = s2.length();   // choose smaller length
 for (; i < n  &&  s1.at(i) == s2.at(i); i++)

(I have also removed unnecessary parentheses, limited the search length to the short string, and changed <= to < because of how array subscripts work.)

Upvotes: 1

juanchopanza
juanchopanza

Reputation: 227578

You are looping beyond the bounds of the string and probably throwing an std::out_of_range exception. Change your condition to

i < s1.length()

and make that check before any calls to std::string::at(size_type pos)

Also, beware your function can only work if s2 is at least as long as s1. You should probably be looping up to one less than std::min(s1.length(), s2.length()).

Upvotes: 2

zzk
zzk

Reputation: 1387

for (; ((s1.at(i) == s2.at(i)) && (i <= s1.length())); i++) {

use i < s1.length()

Upvotes: 0

Related Questions