1

I am intenting sort an array of strings with this code:

void sort(string scadena[]){
    string temp;

    //here i am intenting sort the elements. it works fine

    for(int i=0;i<m;i++){
        for(int j=i+1;j<m;j++){
            if(scadena[i]>scadena[j]){
                temp=scadena[i];
                scadena[i]=scadena[j];
                scadena[j]=temp;    
            }           
        }
    }

    // Here i am intenting remove the repeated elements, but it not works fine.
    for(int i=0;i<m;i++){
        for(int j=0;j<m;j++){
            if(scadena[i]==scadena[j] && j!=i){
                for(int k=j;k <m; k++){
                    scadena[k]=scadena[k+1];
                }
                m--;
            }
        }
    }   

    //Because when i do the cout, the output has repeated elements. it not works
    for(int i=0;i<m;i++){
        cout<<i<<") "<<scadena[i]<<endl;
    }   
}

The output has repeated elements, but i do not why.

The full code has a function that do the permutation of strings.

I do not what happen.

ST3
  • 8,826
  • 3
  • 68
  • 92
Code Geas Coder
  • 1,839
  • 4
  • 23
  • 29

5 Answers5

3

Edit I've just seen this is for homework. Anyway, once you are done with that, here is an idiomatic C++ way to sort a vector of strings, and remove duplicates:

#include <algorithm> // for sort and unique
#include <vector>
#include <string>

....

std::vector<std::string> strings = ....;
std::sort(std::begin(strings), std::end(strings));
auto it = std::unique(std::begin(strings), std::end(strings));
strings.erase(it, std::end(strings);
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
3

The main problem is that when you delete an element from the array, you shouldn't be incrementing the j index, because the string at the current index will have changed, so you need to check it again.

You can fix that by decrementing j at the same time as you decrement m.

Also, it looks like you are overrunning the end of the array in the delete loop.

for(int k=j;k <m; k++){
    scadena[k]=scadena[k+1];
}

Note that when k reaches the last iteration (i.e. k = m-1), you're going to be copy from a position m which is past the end.

The updated loop with both fixes should look like this:

for(int i=0;i<m;i++){
    for(int j=0;j<m;j++){
        if(scadena[i]==scadena[j] && j!=i){
            for(int k=j;k+1 <m; k++){
                scadena[k]=scadena[k+1];
            }
            m--;
            j--;
        }
    }
}
James Holderness
  • 22,721
  • 2
  • 40
  • 52
1

If your sorting works correctly, then you dont need to loop over both i and j to compare strings. You only need to loop over one index and compare with the next string. Then you delete the next string if they are equal, and only increment the index if they are different.

Here are some pseudo code:

int i=0;
while(i+1<m)
  {
    if(scadena[i]==scadena[i+1])
      {
         // Delete scadena[i+1]
         .......
         m--;
      }
    else
      i++;
  }
0

You modify your n loop upper limit in the loop body it may be the reason of your problem,

so remove the line

 m--;

and track the remaining number of string in another variable It's a common good practice when you write a loop of keeping the stop condition stable.

alexbuisson
  • 7,699
  • 3
  • 31
  • 44
0

This should work!

for(int i=0;i<m;i++){
    for(int j=0;j<m;j++){
        if(scadena[i]==scadena[j] && j!=i){
            for(int k=j;k <(m-1); k++){
                scadena[k]=scadena[k+1];
            }
        }
    }
} 
dnawab
  • 113
  • 1
  • 2
  • 8