tags:

views:

198

answers:

6

Hi everyone.

I've been following this forum for sometime now but officially registered today. I'm a Java guy learning C++ now.

As an exercise I'm started to rewrite some of the Java code I had written (while I was learning it ) now in C++.

There are several questions here that helped me a lot. But I'm stuck now and need some help.

The problem I'm trying to solve is to count the max repeating char in an array. The idea is to keep an array of size 26 as a frequency array, arr[0] is for 'a', arr[1] is for 'b' and so on. Increment suitable location for every char in the input array and finally find the index with max count.

This is my Java function :

static char findMax(String str) {
    int arr[] = new int[26];
    int i;

    for(i=0;i<str.length();i++)
        arr[str.charAt(i) - 'a']++;

    int maxFreq = arr[0];
    int maxEleIndex = 0;
    for(i=1;i<26;i++)
        if(arr[i] > maxFreq) {
            maxFreq = arr[i];
            maxEleIndex = i;
        }

    return (char)(maxEleIndex + 'a');
}

And this is my C++ function:

char findMax(string str) {
int *arr = new int[26];
int i;
for(i=0;i<str.length();i++)
    arr[str.at(i) - 'a']++;

int maxFreq = arr[0];
int maxEleIndex = 0;
for(i=1;i<26;i++)
    if(arr[i] > maxFreq) {
        maxFreq = arr[i];
        maxEleIndex = i;
    }   

return (char)(maxEleIndex + 'a');

}

The code compiles fine, but does not give correct output, it reports incorrect char as max repeating. I've used pointer in place of reference, at function in place of charAt. What am I doing wrong?

+7  A: 

You are not initializing the frequency array arr to all zeros in your C++ code.
You can fix this in either of the two ways:

Fix 1:

In Java array elements get initialized to default values depending on their type. For int the default value if 0. In your C++ version the dynamically allocated array elements will have garbage value. Since you want them to be initialized to 0 you should do:

int *arr = new int[26]();
                      ^^

Also note that in C++ unlike in Java you need to deallocate any dynamically allocated memory you allocated. You do it by using the delete operator as:

delete [] arr;

at the end of your function findMax.

Fix 2:

Since you know the size of the array you want to allocate(26) at compile time, there is no need to go for dynamic allocation. You can just do:

int arr[26] = {};

This declares an int array arr with 26 elements all initialized to 0.

codaddict
Or, since `arr` has fixed size, use `int arr[26] = {0};`
larsmans
@larsmans: Was about to add that :)
codaddict
+1 for pointing out failure to delete the allocated array. When I moved from c++ to Java, the GC was such a relief. I shudder at the thought of anyone trying to go in the other direction...
Kevin Day
Thanks codaddict, now it works like a charm.
Eugene
@codaddict `int *arr = new int[26]();` ? I'm sure you aren't allowed to write that according to C++ syntax.
usta
@Kevin Day: Use RAII and forget about the need to explicitly release resources.
usta
@usta: You'll find this answer of help: http://stackoverflow.com/questions/2625551/c-initializing-primitive-array-to-one-value/2625621#2625621
codaddict
@codaddict Doh! I was too blind to not notice that "If the new-initializer is of the form (), the item is value-initialized" in 5.3.4/15 applies to array types as well. Thanks so much for pointing me towards it.
usta
You should also be aware that in C and C++ the expression `'z' - 'a'` is not necessarily equivalent to 25. It could be anything. Only the ten decimal digits are guaranteed to follow each other in the order you expect. Therefore I would suggest that you make the array `UCHAR_MAX + 1` entries long and use `(unsigned char)x` as the index. That also makes the code simpler and doesn't introduce undefined behavior, just in case your test string contains some non-letter.
Roland Illig
Roland Illig
+2  A: 

codaddict is right, you should initialize arr to all zeros and then your code will work. Anyway here's a rewrite to make it look a bit more C++ way

#include <algorithm>
char findMax(const string &str) {  // not necessary to copy the string arg
int arr[26] = { 0 };   // arrays with size known at compile time
                       // can be allocated on stack
for(string::size_type i=0;i<str.length();i++)
    arr[str.at(i) - 'a']++;
return static_cast< char >( std::max_element( arr, arr + 26 ) - arr + 'a' );
}
usta
+1  A: 

If you want to compare with C:

#include <stdio.h>

char findMax (char *str){
    int max, i, maxIndex;

    int arr[26] = {};

    for (i=0; str[i]; i++) arr[str[i] - 'a']++;

    max = arr[0];
    maxIndex = 0;
    for (i=1; i<26; i++){
        if (arr[i]>max){
            max = arr[i];
            maxIndex = i;
        }
    }

    return (char)('a' + maxIndex);
}

int main (){
    char str[51] = "ajsheuaptoemandjeitmaneitmaneowpqlakemtnatengjfnau";

    printf ("char: %c\n", findMax (str));

    return 0;
}
GagleKas
+1  A: 

Eugene

You can put below line in your code to assign value ZERO to the arr. or better avoid using dynamically allocation as it is not required for you and deallocation needs to be done manually( Its not like java)

Filling the Block of Memory:

memset(arr,0,26*sizeof(int)); //In findMax after allocating memory to arr

Deallocation:

delete [] arr; //In findMax function before return

+1  A: 
#include <algorithm>
#include <iostream>
#include <climits>
#include <string>

using std::string;

char findMax(const string &str) {
  string::size_type freq[UCHAR_MAX + 1] = { 0 };

  for (string::const_iterator it = str.begin(); it != str.end(); ++it)
    freq[(unsigned char) *it]++;
  return static_cast<char>(std::max_element(freq, freq + UCHAR_MAX + 1) - freq);
}

int main() {
  char letter = findMax("Hello, world. How are you?");
  std::cout << letter << "\n";
  return 0;
}

Some remarks:

  • In C++, there is a tendency to use iterators instead of explicit array indexes.
  • This code works no matter which charactersbytes appear in the string. So beware of Unicode characters.
  • Since a char may be either signed or unsigned, I had to convert the array index to something that is guaranteed to be nonnegative.
Roland Illig
+1  A: 
char findMax(const string& str) 
{
    const int alphabet_size = 26;

    int arr[alphabet_size] = {};

    for_each(str.begin(), str.end(), [&](char c) { ++arr[c - 'a']; });

    return max = max_element(arr, arr+alphabet_size) - arr + 'a';
}
Grozz