Find none duplicated letter in a char array. My program works wrong. Need help

Find none duplicated letter in a char array. My program works wrong. Need help

#include <iostream>
#include <vector>
#include <list>
# include <ext/hash_map>

/*
find none duplicated letter from char array */

using namespace __gnu_cxx;
using namespace std;

list<char> find_Noneduplicate_idx(const vector<char>& A)
{
hash_map<char,int> X;
list<char> idx;
for ( int i = 0; i < A.size(); ++ i ) 
{
  hash_map<char, int>::iterator it = X.find(A[i]);
  if ( it != X.end() )  
  {
    X[A[i]]++;
  }
  else
  X[A[i]] = 1;
}
for (hash_map<char,int>::iterator iter = X.begin(); iter!=X.end();++iter)
{
    if (iter->second == 1)
        idx.push_back(iter->first);
}

return idx;
}

int main()
{

    int myints[] = {'H','A','C','B','C','H','Z'};
    vector<char> fifth (myints, myints + sizeof(myints) / sizeof(char) );
    list<char> idx = find_Noneduplicate_idx(fifth);
    for (list<char>::iterator iter = idx.begin(); iter != idx.end(); ++iter)
    {
        printf("%c",*iter);
    }
    return 0;

}

The output should be : A,B,Z but my result is not.

I edited开发者_开发知识库 it, now the output is "AB?Z`??" what's wrong?

First of all, change this

int myints[] = {'H','A','C','B','C','H','Z'};

to

char myints[] = {'H','A','C','B','C','H','Z'};

Otherwise when creating the vector on the following line, you're reading past the end of this array. Try std::cout << fifth.size() to see just how big your vector became!

Or, alternatively, change sizeof(char) to sizeof(myints[0]) -- it is good practice to use names, rather than types, with sizeof in any case.

Then add the missing else keyword before X[A[i]] = 1;, and the program will produce the expected result.

for ( int i = 0; i < A.size(); ++ i ) 
{
  hash_map<char, int>::iterator it = X.find(A[i]);
  if ( it != X.end() )  
  {
    X[A[i]]++;
  }

  X[A[i]] = 1; //Shouldn't this be in an else?
}

Not answering the question but commenting on code:

int myints[] = {'H','A','C','B','C','H','Z'};
vector<char> fifth (myints, myints + sizeof(myints) / sizeof(char) );

Why do you have an array of int's which are all letters. Why not a string or an array of char?

list<char> idx = find_Noneduplicate_idx(fifth);

Why pass a specific container. Why not pass the begin and end iterators then you can use any container. Even the original int array without needing to copy it into a vector.

for (list<char>::iterator iter = idx.begin(); iter != idx.end(); ++iter)
{
    printf("%c",*iter);
}

If you are going to use C++ use the std streams. They are much safer in the long run. The above line can then be trivally implemented as:

std::copy(idx.begin(), idx.end(), std::ostream_iterator<char>(std::cout));

This can be simplified:

hash_map<char, int>::iterator it = X.find(A[i]);
if ( it != X.end() )  
{
  X[A[i]]++;
}
else
  X[A[i]] = 1;

as

X[A[i]]++;

If the operator[] does not find the element in the array it automatically creates and initializes it using the T(). Which means for the int type int() which initializes the value to zero. Then performing the addition will make it one the first time.

Rather than doing this:

using namespace __gnu_cxx;
using namespace std;

explicily prefix things with std:: it will avoid confusion in the long run.

This part is wrong:

sizeof(myints) / sizeof(char)

myints is an array of integers.
Prefer to use this:

sizeof(myints)/ sizeof(myints[0])