Deleting duplicates in an array (C++)

Question

I saw an older post on here asking how to do relatively the same thing, but their approach was different and i'm interested to know the hole in my program.

I am attempting to write a program that accepts characters into a 10 character length array. I want the program to evaluate the first array position and delete any duplicates it finds later in the array by identifying a duplicate and moving all of the values to the right of it to the left by one. The 'size' of the array is then decreased by one.

I believe the logic I used for the delete function is correct but the program only prints an 'a' for the first value and the fourth value in the array.

Any help would be greatly appreciated, here is my code:

#include <iostream>
using namespace std;

int letter_entry_print(int size, char array[10]);
int delete_repeats(int& size, char array[10]);
int final_array_print(int size, char array[10]);

int main()
{
    char array[10];
    int size = 10;

    letter_entry_print(size,array);
    delete_repeats(size,array);
    final_array_print(size,array);

    cout<<"\n";
    system("pause");
}

int letter_entry_print(int size, char array[10])
{
    int i;

    for (i=0;i<size;i++)
    {
        cout << "Enter letter #" << i+1 << endl;
        cin >> array[i];
        cout << "\n";
    }

    cout << "\nYour array index is as follows:\n\n";

    for (i=0;i<size;i++)
    {
        cout << array[i];
        cout << " ";
    }

    cout <<"\n\n";
    return 0;
}

int delete_repeats(int& size, char array[10])
{
    int ans;
    int loc;
    int search;
    int replace;
    char target='a';

    cout << "Enter 1 to delete repeats.\n\n";
    cin >> ans;
    if(ans==1)
    {
        for(loc=0;loc<size;loc++)
        {
            array[loc]=target;
            for(search=1;search<(size-loc);search++)
            {
                if(target=array[loc+search])
                {
                    for(replace=0;replace<(size-(loc+search));replace++)
                    {
                        array[loc+search+replace]=array[loc+search+replace+1];
                        array[size-1]=0;
                        size=(size-1);
                    }
                }
            }
        }
    }else(cout<<"\nWhy didn't you press 1?\n\n");
    return 0;
}

int final_array_print(int size, char array[10])
{
    cout<<"\nYour new index is as follows:\n\n";
    int i;
    for(i=0;i<size;i++)
    {
        cout<<array[i];
        cout<<" ";
    }

    cout<<"\n";
    return 0;
}

if(target=array[loc+search]) Are you sure you want to do an assignment here ? I think you should use, == operator. Using std::string and STL algorithms, this can be done a lot easier. — Mahesh
– Mahesh, Commented Sep 8, 2012 at 5:31

Arunas · Accepted Answer · 2012-09-08 05:54:19Z

1

Ok, there are a few things about your code that look odd.

1) you repeat 10 all over the place to the point where there's no way you could resonably change it, but you also pass size along. Instead of making all your functions take arrays of 10 chars, consider just passing in a pointer to char, like:

int final_array_print(int size, char *array)

then you can change the size of your arrays more easily. There's no point in passing size everywhere if you're going to limit yourself forever to 10 items, and there's no good reason to pass arrays of 10 items around if you provide a size!

2) ok, so now you want to look for duplicates. Why do you overwrite the first element in your array with an 'a'?

 char target='a';
...
array[loc]=target;

wouldn't you want to do it the other way around?

3) next, as @Mahesh points out, you probably want to use the comparison operator '==' rather than the assignment operator = when looking for duplicates that is:

if(target=array[loc+search])

should probably be

if(target == array[loc+search])

4) Next, dontbeafraidtousealittlewhitespacebetweenyourwordsandpunctuation.Itmakesitaloteasiertoidentifytypingmistakesandspellingerrors.

5) your loop to actually perform the replacement has incredibly complicated indices. It would be easier if you didn't start with replace = 0, but just start at replace = search + 1, try it out and perhaps you'll how much simpler all the rest of the indices become.

answered Sep 8, 2012 at 5:54

Arunas

1,3321 gold badge11 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

codinglocc Over a year ago

Thanks so much for the help. I got my code to work, the missing '=' was a culprit along with the char target initialized to a. I did that because when I didn't set it to something and just wrote 'char target;', the compiler would complain that I was using the variable 'target' without initializing it, so I thought I would initialize it to a random letter and the function would overwrite it, but the 'a' ended up overwriting the subsequent data. Do you know why it would say this? I thought declaring 'char target' WAS initializing it.

Arunas Over a year ago

declaring target to be a char does not initialize it. It could be anything until you actually put something it it. Just declaring 'target' to be a 'letter' doesn't tell anybody which letter it is.

Collectives™ on Stack Overflow

Deleting duplicates in an array (C++)

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related