Finding duplicates in a user defined array using a function

[smilesofmiles]

Hello, I'm extremely new to C++ and will appreciate any help you would be willing to offer. I need to create a function that will find the duplicate characters in a string and then when a duplicate is found, it will break out of the search loop and print the position of the character that was a duplicate.
For example, if the user inputted "Coconuts" the program would print 3 because c has a duplicate at position 3. It wouldn't print any other duplicates because 3 the character c found it's duplicate before any other character did.

My main two questions are:
1. How do I correctly store the length of a user defined array inside of an integer?
2. Should I be using pointers?

Please let me know if I've broken any forum rules or if I need to be more clear, also if there are any problems with my logic. :-( It probably looks like a logical disaster.

Here's what I have so far:

#include <iostream>
#include <array>
#include <string>
using namespace std;

int main(){

	char strarr[2000]; // I made this a large number in case the user enters a huge string.
	int i = 0;
	int j = 0;
	int len = strlen(strarr); // I'm trying to store the length of the character array inside of the integer variable len.

	cout << "Please enter a string." << endl;

	cin.getline(strarr, 2000); 

void findDuplicate(char strarr){

	while (strarr != '\0'){

		for (i = 0, i < len, i++){

			for (j = 0, j < len, j++){

				if (j = i){
					cout << strarr[i];
				}
				else
					cout << "No duplicate indexes found";
			}

		}
}
			return 0;		}

Thank you in advance!

 

[johng1]

Hi,
Unfortunately, your program is riddled with errors. As you know, a C++ program first needs to be compiled. So when you write an app, the first thing to do is to compile it. The compiler will tell you of errors. Unfortunately, these error messages can be kind of terse and particularly hard for a beginner to understand. Let me go through your program a line at a time and point out the difficulties:

#include <iostream>
#include <array>
/* The include directive includes class declarations. But there is no class array
in C++. Arrays are predefined in C and hence also in C++.
*/
#include <string>
/* This directive allows the program to use class string. However, the function
that you want to use is not a method of this class. To use function strlen you
must have: #include <string.h>. This directive allows you to use all C functions
for C strings; i.e null terminated arrays of characters.
*/
using namespace std;

int main(){

char strarr[2000]; // I made this a large number in case the user enters a huge string.
/* This declares strarr as an array of 2000 characters. The individual
char components of the array are random. In particular, there may be no
null character among the components. So usage of strln(strarr) is an error.
*/
int i = 0;
int j = 0;
int len = strlen(strarr); // I'm trying to store the length of the character array inside of the integer variable len.
// see above for the error
cout << "Please enter a string." << endl;

cin.getline(strarr, 2000);
/* C++ does not allow nesting of functions. You must move this function outside of
main. Then call this function from main. You can either move the function above
main or give a function prototype. Assuming the function has been moved:
*/
void findDuplicate(char strarr){
/* The parameter strarr is a single char, not an array of char, which is what you
want. You can declare strarr to be an array in the function heading by
char strarr[]
To use int variables i, j and len you must declare them inside this function.
Say int i,j,len;
You can then write len=strlen(strarr); However, if the array strarr is not a C
string, this may lead to error
*/

while (strarr != '\0'){
/* Now that strarr is an array, you are comparing an array to 0. Turns out that
strarr != '\0' is always true! So this will be an infinite loop. Easiest fix:
just remove the while loop.
Now you want to examine each of the chars of strarr starting at strarr[0] to see
if any of the following chars are the same as the examined char.
*/
for (i = 0, i < len, i++){
/* The 3 expressions of a for loop are separated with semii colons, not commas. You need to correct it
here and also in the "for j loop" below.
*/
for (j = 0, j < len, j++){
/* You want to know if a component of strarr after the ith component is the same as
the ith component. That is, for a j>i, is it true that strarr==strarr[j]? So
you should change the start value of j to i+1; i.e.
for (j = i+1;j<len;j++)
*/
if (j = i){
/* You want to know if the component at index j is the same as the component at index
i, not whether index i is is the same as index j. Furthermore, you have the
assignment operator =, not the comparison operator == that you want. So
change this to: if (strarr[j] == strarr) {
*/
cout << strarr;
/* You want to output the index j where a duplicate occurs, not the actual duplicate
char. So change this to cout << j;
Furthermore, if the condition is true, you've found a duplicate and the function's
job is complete. So after the output of the index, insert the return; statement.
*/
}
else
cout << "No duplicate indexes found";
/* You really don't want to output the string every time a non duplicate is being
examined. So remove the else clause.
*/
}

} // end of corrected for j loop
} // end of corrected for i loop
/* Here is where you want to output the info that no duplicates found.
} // end of function.
return 0; // this is in main
}// end of function main.

 

[smilesofmiles]

Wow, thank you so much! You've given me a lot of helpful information. Hopefully I got all of it down. I used a notebook on the side to make sure I was working through it properly. :-) I figured out that I also didn't understand how to call functions so I watched some videos and I got it taken care of.
Thank you again for the time and effort you put in for helping me!

 

[johng1]

Hi again,
I hope you were able to use my previous post to write a correct program. Since you're just starting C++, maybe it was expected that you use the string class. Here's a complete program that does just that:

#include <iostream>
#include <string>
using namespace std;

void findDuplicate(string s) {
   int i,pos;
   for (i = 0; i < s.length(); i++) {
      pos=s.find_first_of(s[i],i+1);
      if (pos != string::npos) {
         cout<<pos<<endl;
         return;
      }
   }
   cout << "No duplicate indexes found"<<endl;
}

int main(int argc, char** argv){
   char strarr[2000]; // I made this a large number in case the user enters a huge string.
   cout << "Please enter a string." << endl;
   cin.getline(strarr, 2000);
   string s(strarr);
   findDuplicate(s);
   return 0;
}

 

[smilesofmiles]

My code almost does what I want it to do. It displays the position of the duplicate but does the last duplicate position instead of the first. However, I didn't know there was a function called find_first_of! I was trying to use break to exit out of the loop after finding the first duplicate and didn't catch that it wasn't doing that. I wasn't using the right words to test my code with. Your code was a lot of fun to work through and now I understand npos, find_first_of and int main(int argc, char** argv). After seeing your code though, I may just use find_first_of and npos for any more problems that I encounter with matching or duplicates.

//Here is the code you helped me with. :-)
Thank you again!
[CPPS]
#include <iostream>
#include <stdio.h>
using namespace std;

void findDuplicate(char strarray[]){
int i, j, len;
len = strlen(strarray);

for (i = 0; i < len; i++){
for (j = i + 1; j < len; j++){
if (strarray[j] == strarray) {
cout << j << endl;
return;
break; // I was trying to make it break from all the loops.
}
}
}
cout << "No duplicates found." << endl;
}


int main(){

char strarray[2000];

cout << "Please enter a string." << endl;

cin.getline(strarray, 2000);

findDuplicate(strarray);

return 0;
}
[/CPPS]

 

[johng1]

The problem of breaking completely out of nested loops has a couple of solutions. The first solution is to introduce a boolean variable found which is initially false; make the controlling statements of loops to include the expression "&& !found". Then when the break out should occur, set found to true. This is the "structured programming" solution.
The second solution is simpler; it uses goto somelabel;. The goto statement immediately transfers control to the statement following somelabel: I agree with most programmers who say usage of goto should be avoided. The problem is that numerous goto statements make the program hard to understand; it's not structured. A good program should be almost self explanatory.
Here's two further functions for your problem:

void findDuplicate1(char strarr[]) {
   int i, j, len;
   bool found = false;
   len = strlen(strarr);
   for (i = 0; i < len && !found; i++) {
      for (j = i + 1; j < len; j++) {
         if (strarr[i] == strarr[j]) {
            cout << j << endl;
            found = true; // when we get back to top of for i loop, it's finished
            break; // go to end of the for j loop
         }
      }
   }
   if (!found) {
      cout << "no duplicates found" << endl;
   }
}

void findDuplicate2(char strarr[]) {
   int i, j, len;
   for (i = 0; i < len; i++) {
      for (j = i + 1; j < len; j++) {
         if (strarr[i] == strarr[j]) {
            cout << j << endl;
            goto alldone;
            // this transfers control to the statement following label alldone
            // i.e. we break out of both loops
            // goto is usually a bad idea in programming; in this case it has
            // the exact same effect as return
         }
      }
   }
   cout << "no duplicates found" << endl;
alldone:
   ;
}