Deleting string elements in C++

Question

I have to input a integer number, and then a a string with n elements, int type; The task is to delete the repeating elements. I tried to resolve the problem with this code, but don't know where is the mistake. Can you help me?

#include <iostream>
#include <string>

using namespace std;
int main()
{
    int n, br=0;

    string masiv;
    cin>>n;
    for(int i=0;i<n;i++)
    {
        cin>>masiv[i];
    }
    n=masiv.size();
    for(int i=0;i<n;i++)
    {
        for(int j=0;j<n;j++)
        {
            if(masiv[i]==masiv[j] && i!=j)
            {
                masiv.erase (std::remove(masiv.begin(), masiv.end(), masiv[i]), masiv.end());
            }
        }
    }
    cout<<masiv<<endl;
    system("pause");
    return 0;
}

The most general (for example, works with const string too) way is to do the opposite of removing parts: copy the parts you do want into a new string in correct order, instead of repeatedly modifying the source string. This also applies if you use vector or whatever, instead of a string. — hyde, Dec 03 '13 at 08:49
You are using a string to store integer values. Is that logical? You can refer to this link for your task [Most efficient way to erase duplicates and sort a c++ vector?](http://stackoverflow.com/questions/1041620/most-efficient-way-to-erase-duplicates-and-sort-a-c-vector) — Deidrei, Dec 03 '13 at 08:53
Your program has lots of issues. A `std::string` always stores `char`, though you can instantiate `basic_string` or use a `vector` to store `int`s. Given `char` is normally an `8` bit `int`, you can store numbers into it but it may not work as expected outside the range 0..127. You need to add to the string using `+=`, not index ala `[i]` to locations that don't yet exist. — Tony Delroy, Dec 03 '13 at 09:01
The other big issues in getting your code working: 1) you remove elements inside your loops, but always use j++ or i++ to jump over the index just considered - if you'd done a deletion a new element would have slotted in there and you'd skip it; 2) the size() changes when you delete elements, so you keep processing the erased elements - in this case the `vector` `capacity()` will be enough for these off-the-end accesses, but an implementation may still check for this and abort/throw etc.. — Tony Delroy, Dec 03 '13 at 09:09

T-D · Accepted Answer · 2013-12-03T09:00:50.333

2

Use this code instead :

    #include <iostream>
    #include <string>

    using namespace std;
    int main()
    {
        int n, br=0;

        string masiv="";
        cin>>n;
        for(int i=0;i<n;i++)
        {
            char c;
            cin>>c;
            masiv += c;
        }
        n=masiv.size();
        string result="";
        for(int i=0;i<n;i++)
        {
            bool repeated = false;
            for(int j=0;j<result.size();j++)
            {
                if(masiv[i]==result[j])
                {
                    repeated = true; // indicating that massiv[i] was added before
                }
            }
            if(repeated == false)
            {
                result += masiv[i]; // won't add masiv[i] to the result unless it's the first time encountered 
            }
        }
        cout<<result<<endl;
        system("pause");
        return 0;
    }

edited Dec 03 '13 at 09:00

answered Dec 03 '13 at 08:55

T-D

1,306
1
16
23

The existing code implies that repeating elements should be output once, rather than discarded altogether, but maybe it's wrong there given the question wording is consistent with what you've done...? – Tony Delroy Dec 03 '13 at 09:06
But that way keeps one of the repeating elements. I need to erase all elements, that are repeating minimum twice – Legeran Dec 03 '13 at 09:06
This answer doesn't delete elements in a string, just ignores the repeated ones and creates a new string of the non-repeated elements. Which is also irrelevant to the OP's question. Not to mention it does not do any explanation about the code fixes, therefore -1. – Varaquilex Dec 03 '13 at 09:13

Varaquilex · Answer 2 · 2013-12-03T09:26:58.560

First, you should read the input into a character and use push_back() to save the read character into the string.

cin>>n;
for(int i=0;i<n;i++)
{
    char c;
    cin>>c;
    masiv.push_back(c);
}

You are getting segmentation fault because you save the size of the string into value n (n=masiv.size();) which you shouldn't because once you erase an element from the string, its size changes. Therefore you should use size() method for the loop boundary as below.

for(int i=0;i<masiv.size();i++)
{
    for(int j=0;j<masiv.size();j++)
    {
        if(masiv[i]==masiv[j] && i!=j)
        {
            masiv.erase(j, 1);
            j--;
        }
    }
}

See how smiple erase() method is? Just erase the repeated element (latter one). j specifices the indice of the repeated element and 1 specifies the character count that will be deleted starting from that indice. You have to decrement j because once you erase the character at the jth indice, now j will be pointing to the next character. When inner loop continues it will increment j. If you do not adjust the indice after erasing a character, you will skip a character when the loop increments. You have to adjust j to point to the correct index. Therefore j--;.

Edit: Better loops

You can re-arrange your loops in a way that you do not have to check whether i==j or not. Just start the inner loops index just 1 larger than that of outer loops.

for(int i=0;i<masiv.size();i++)
{
    for(int j=i+1;j<masiv.size();j++)
    {
        if(masiv[i]==masiv[j])
        {
            masiv.erase(j--, 1);
        }
    }
}

Assume you have entered level as the string. The outer loop starts from l and inner loop starts from e. By this way you make less iterations and guarantee not to compare same indices.

score 0 · Answer 3 · answered Dec 03 '13 at 09:59

#include <iostream>
#include <string>
#include <algorithm>    // std::unique, std::distance
#include <cassert>

int main(int argc, char** argv) 
{
    std::string masiv;
    int n; 
    std::cin >> n;
    assert( n > 0);
    std::cin.ignore(10,'\n');   // ignore 

    std::getline(std::cin,masiv);
    masiv.resize(n);

    std::string::iterator it = std::unique (masiv.begin(), masiv.end());
    masiv.resize( std::distance(masiv.begin(),it) );

    std::cout << masiv << std::endl;

    return 0;
}

Deleting string elements in C++

3 Answers3