1

I wrote the following code for removing the duplicates from a given string i.e. if ARRUN is the input then the output will be ARUN.

#include <bits/stdc++.h>
using namespace std;
char* removeDuplicates(string &s,int n){
    char arr[n];
    unordered_map<char,int> exists;
    int index = 0;
    for(int i=0;i<n;i++){
        if(exists[s[i]]==0)
        {
            arr[index++] = s[i];
            exists[s[i]]++;
        }
    }
    return arr;
}

//driver code
int main(){
    string str;
    cin >> str;
    cout<<removeDuplicates(str,str.length())<<endl;
    return 0;
}

The code produces no output at all, however, it works fine if I use char arr[] instead of string class.

Improve this question
4
  • 4
    You cannot return an automatic (non static) array from a C++ function. Arrays are not first class objects in C++. Commented Feb 16, 2020 at 11:07
  • @Arun Suryan Do you need to remove adjacent duplicate characters or all duplicate characters? Commented Feb 16, 2020 at 11:11
  • All duplicate characters. Commented Feb 16, 2020 at 11:13
  • 2
    You should never #include <bits/stdc++.h>. It is not just bad practice - it is not proper C++. It ruins portability and fosters terrible habits. By using it, you not only grant the compiler the right to break your code at any time without notice but also renders your code non-portable and unprofessional. It also creates implicit dependency on any future facility of the C++ standard library, thus basically screwing up compile time. See Why should I not #include <bits/stdc++.h>. Commented Feb 16, 2020 at 12:34

8 Answers 8

Reset to default
4

You can't use char arr[n] without being n constant or constexpr.

You don't need map. set is sufficient.

Note that map and set remove duplicates already, then you can check if any element is inserted or not to get your new string in the same order of the first, as follows

#include<string>
#include<iostream>
#include<unordered_set>

std::string removeDuplicates(const std::string &s){
    std::string arr;
    std::unordered_set<char> exists;

    for(const auto&el:s)
        if(exists.insert(el).second) arr+=el;

    return arr;
}

//driver code
int main(){
    std::string str;
    std::cin >> str;
    std::cout<<removeDuplicates(str)<<std::endl;
    return 0;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

I loved your coding style. But I am just too lazy to write std:: again and again. bits/stdc++.h gets the job done. Though, I know it's not a good practice.
@ArunSuryan except only one compiler line got that header
@ArunSuryan bits/stdc++.h gets the job done -- Please read this
It's very crucial in sports programming. That's why I often use it. It saves typing effort.
Also read this. Your "sports programming" header file will fail miserably if you ever write code that uses data as a struct type or similar.
|
2

std::string support removing elements.

#include <iostream>
#include <string>

std::string removeDuplicates(std::string str) {
    for (int i = 0; i < str.size(); i++) {
        while (true) {
            int j = str.find_last_of(str[i]);
            if (i < j) {
                str.erase(j, 1);
            } else {
                break;
            }
        }
    }
    return str;
}

int main() {
    std::cout << removeDuplicates("ARRUN");
    return 0;
}
Improve this answer

Comments

1

If a function declaration looks the following way

char* removeDuplicates(string &s,int n);

then it means that the passed object itself will be changed in the function. Otherwise the parameter shall have the qualifier const.

Also it is unclear why the function has return type char *. It looks like the declaration of the function is contradictive.

The second parameter of the function shall have at least the type size_t or that is better std::string::size_type. The type int can not accomodate all values of the type std::string::size_type.

The function could be declared without the second parameter.

A straightforward approach without using intermediate containers that requires dynamic memory allocation can look the following way

#include <iostream>
#include <string>

std::string & removeDuplicate( std::string &s )
{
    const char *p = s.c_str();

    std::string::size_type pos = 0;

    for ( std::string::size_type i = 0, n = s.size(); i < n; i++ )
    {
        std::string::size_type j = 0;
        while ( j < pos && s[i] != s[j] ) j++;

        if ( j == pos )
        {
            if ( i != pos ) s[pos] = s[i];
            ++pos;
        }
    }

    return s.erase( pos );
}

int main() 
{
    std::string s( "H e l l o" );

    std::cout << "\"" << s <<"\"\n";

    std::cout << "\"" << removeDuplicate( s ) <<"\"\n";

    return 0;
}

The program output is

"H e l l o"
"H elo"
Improve this answer

Comments

1

@Arun Suryan, You pointed out correctly. But you can do it without using vector by using global char array.

Also don't forget to append the newline at the end!

Have a look at the following code:

#include<string>
#include<iostream>
#include<unordered_map>

char* removeDuplicates(std::string &s,int n){

    std::unordered_map<char,int> exists;
    char* arr = (char*)(malloc(n*sizeof(char)));
    int index = 0;
    for(int i=0;i<n;i++){
        if(exists[s[i]]==0)
        {
            arr[index++] = s[i];
            exists[s[i]]++;
        }
    }
    arr[index] = '\n';
    return arr;
}

//driver code
int main(){
    std::string str;
    std::cin >> str;
    std::cout<<removeDuplicates(str,str.length())<<std::endl;
    return 0;
}
Improve this answer

7 Comments

Yes you can, but returning values through global mutables without real need to do so is Fortran-77 style of programming (because Fortran had no other way). vector uses heap, which is global but symbolically the manager object got certain locality and identity. In large projects use of global static mutables becomes a real menace which is pointed out by C++ and similar imperative languages opponents.
@Swift-FridayPie Agreed!
Sorry, I have to downvote your answer, because it promotes the usage of global variables and raw C arrays with fixed size (thus leading to a guaranteed buffer overflow), neither of which is compatible with modern C++ programming practices. Furthermore, this approach significantly increases the difficulty of code analysis ("Did some function just modify that global variable?"), letting alone introducing data races. Consistently use std::string and value semantics instead. And that not even counting #include <bits/stdc++.h> using namespace std;.
@L.F. No worries! Suggestions taken!
malloc is just as bad. You should be using std::string instead of a manually allocated (or fixed sized) char array. In fact you are leaking the allocation right now and malloc without placement-new is technically (currently) always undefined behavior. There is no reason to use it over new[].
|
1

This might be a bit advanced for newcomers to C++ but another solution makes use of the erase-remove idiom:

std::string removeDuplicates(const std::string& s) {
    std::string result = s;
    std::unordered_set<char> seen;

    result.erase(std::remove_if(result.begin(), result.end(), [&seen](char c)
        {
            if (seen.find(c) != seen.end())
                return true;

            seen.insert(c);
            return false;
        }),
    result.end());

    return result;
}

It basically uses a set to store characters that have been seen, shuffles the characters to be removed down to the tail (using std::remove_if) and erases the tail from the string.

Working version here.

Improve this answer

Comments

1

This works too, a single line solution with an inbuild function.

cout<<str.erase(std::unique(str.begin(), str.end()), str.end());

Improve this answer

Comments

0

Simple Answer

#include<bits/stdc++.h>
using namespace std;
string removeduplicate(string key){
    set<char>s;
    string ans="";
    for(int i=0;i<key.size();++i){
        if(s.find(key[i])==s.end()){
            s.insert(key[i]);
            ans.push_back(key[i]);
        }
    }
    return ans;

}



int main()
{
    string key="";
    cout<<"enter the key:";
    cin>>key;
    string ans1=removeduplicate(key);
    cout<<ans1;
    return 0;
}
Improve this answer

Comments

-1

So, after doing a bit of reading on the Internet I realized that I was trying to return a pointer to the local array in the removeDuplicates() function.

This is what works fine 

#include <bits/stdc++.h>
using namespace std;
void removeDuplicates(string &s,int n){
    vector<char> vec;
    unordered_map<char,int> exists;
    int index = 0;
    for(int i=0;i<n;i++){
        if(exists[s[i]]==0)
        {
            vec.push_back(s[i]);
            exists[s[i]]++;
        }
    }
    for(auto x: vec)
        cout << x;
}

//driver code
int main(){
    string str;
    cin >> str;
    removeDuplicates(str,str.length());
    return 0;
}

PS: We can make the return type of function to vector as well.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.