CODEWITHSUNDEEP
c++arrays
cake_lord_97
cake_lord_97

Reputation: 51

Implementing common char count in C++ for 2 strings but receiving vastly different char count for the same for loop on 2 strings

I'm trying to get an output something like for the first for loop where I cout << char << charcount but for my second loop, I'm getting very large counts and my string became "testfr" from my original input string "tester". I'm not sure whats went wrong.

This is the code I have.

int commonCharacterCount(string s1, string s2) {

    int map1[26] = {};
    int map2[26] = {};
    int common_chars = 0;

    for (size_t i = 0; i < s1.size(); i++){
        map1[s1[i]]++;
        cout << s1[i] << ": " << map1[s1[i]] << endl;
    }

    cout << "---------------------------" << endl;

    for (size_t j = 0; j < s2.size(); j++){
        map2[s2[j]]++;
        cout << s2[j] << ": " << map2[s2[j]] << endl;
    }

    /*for (int k = 0; k < 27; k++){
        common_chars = common_chars + min(map1[s1[k]], map2[s2[k]]);
    }
    cout << "common chars: " << common_chars << endl;
    */
    return common_chars;
} 

int main(){
    string a = "test";
    string b = "tester";
    commonCharacterCount(a,b); 
}

And this is the output I'm getting:

t: 1
e: 1
s: 1
t: 2
---------------------------
t: 16652017
e: 29286
s: 1
t: 16652018
f: 1
r: 9

Upvotes: 0

Views: 68

Answers (1)

selbie
selbie

Reputation: 104569

This line:

map1[s1[i]]++;

s1[i] will typically be an ascii value between a and z. The ordinal values for chars in the a-z range are between 97 and 122. 65 - 90 for capitol letters. And other printable characters and digits will have ordinal values greater than 31. But the only valid indices in your array are between 0 and 25.

Hence, undefined behavior since your code is now writing bytes from the string into random places in memory.

Probably the simplest fix is this.

Declare your map arrays to hold 256 values. Enough for any 8-bit char.

int map1[256] = {};
int map2[256] = {};

You could also use UCHAR_MAX from <climits> instead of hardcoding 256.

Then assign as follows:

for (size_t i = 0; i < s1.size(); i++){
    unsigned char uc = (unsigned char)(s1[i]);
    map1[uc]++;
    cout << s1[i] << ": " << map1[uc] << endl;
}

Another simple alternative is to use the std::unordered_map class in C++:

std::unordered_map<char, unsigned int> map1;
for (char c : s){
   map1[c]++;
   cout << c << ": " << map1[c] << endl;
}

Upvotes: 1

Related Questions