Array now showing

[JohnnyJoestar21]

Hello all, I am doing a code to run to check for duplicates generated randomly.
my Issue is that this code doesn't print the elements if I ask the user to enter a specified range from 1-100.

Here is my code. (The code works without asking for a range and defining an array from the beginning).

  #include <iostream>
#include <ctime>
#include <string>

using namespace std;


int main() {
	srand(time(0));
	int arr[100];
	int dup, n, random;
	cout << "=============Find the dups in an Array=============\n \n";


	cout << "Enter the size of the array: "; cin >> n;

	for (int i = 0; i < n; ++i) {
		random = rand() % 100 + 1;
		arr[i] = random;
		dup = arr[i];
		for (int j = i + 1; j < n; ++j) {
			if (dup == arr[j]) {

				cout << dup << " ";

			}

		}
	}
}

[dutch]

You're looking the wrong way for the duplicates. You need to look from 0 up to i, not i + 1 to n.

[JohnnyJoestar21]

Sir, I believe it is the correct way since I already have a for loop checking from 0 up to i I ran a nested for loop to check for duplicates using j and j is i+1 up to n.

[lastchance]

@JohnnyJoestar21, how can you possibly check for duplicates amongst elements that you haven't set yet? When you get to i=10, for example, you will only have set values for elements 0,1,...,10 - so you can't possibly check against elements 11, 12,...

The direction in your method would only work if you set the whole array before checking for duplicates, not if you are checking as you assign them. As @dutch pointed out, you are looking the wrong way for duplicates.

Variable random is also unnecessary; you can condense lines 18 and 19.

Amongst your headers, you don't need <string>, but you do need <cstdlib>.

[markyrocks]

I played around with a similar concept awhile back. You need to pick a random number, then loop through all the existing elements in the arr array to check if the number is already in there before you add it in. It looks like youre setting dup = arr[i] and then just comparing the rest of the array to dup which hasn't been set yet?

for instance of arr[5] = dup.... or vice versa 5 being the current position of the last good saved number. you are comparing dup to arr[6] to [99].

this would be my take on it

int main() {

    int random;  //pretend random number
    int arr[100];
    bool boo = true;

    for (int i = 0; i < 99; i++) {

        random = random; // pick random

        for (int x = 0; x < i; x++) {
            if (arr[x] == random) {
                //dup
                boo = false;
                break;
            }
            else {
                boo = true;
            }
           
        }
            
        if (boo) {
          arr[i] = random;
         }
        else {
         i--;
        }
    }

}

I'd also like to mention that the above maybe complete crap bc im tired rn.

i'd also like to add that letting the user pick the number of the max array elements is a very bad idea. you need to at least put in a check in there to make sure the array doesn't go out of bounds. especially since 100 is very low.

[JohnnyJoestar21]

@lastchance, Thank you for you contribution, but to be honest I can't seem to understand what you are trying to say. the string header is just there because I was doing programming with it before I started this program. The program was working fine until I added the cin>>n;

[lastchance]

The cin >> n; is irrelevant. It will work exactly the same if you write, say, n = 50;

I suspect that you had a previous working code where you set ALL of the array elements first ... and then you went back to check for duplicates. This would be fine, but is less efficient.

Here, you have a code which is checking for duplicates WHILST you are setting the array elements. Your fault - which three of us have pointed out to you in our ways - is that you are checking against elements THAT YOU HAVEN'T SET.

So start with the outer loop and the first pass i=0

You set arr[0] to a random number.

And then, still within the i=0 loop, you proceed to check this value against arr[1], arr[2], arr[3], ... , arr[n-1].

But you HAVEN'T YET SET ANY OF arr[1], arr[2], arr[3], ... because they will be set with later values of i.


You simply need to change
for (int j = i + 1; j < n; ++j) {
to
for (int j = 0; j < i; ++j) {

Then run with a large enough value of n to give a good chance of getting any duplicates; say n = 50 or larger.


Ignore me if you choose, but you should fix your headers.

[JohnnyJoestar21]

I did what you have done, and changed my headers but now it's printing the array only.



EDIT: I have fixed the code and it works now.

[lastchance]

Here is your code with corrected headers and corrected j loop.

Run it in cpp.sh (button to the right of the code).

Enter 50 for n when requested. It will print any duplicates. There will inevitably be some, but not 50 of them. You can also try with smaller numbers for n. Very small numbers may not give duplicates. Large numbers give lots of duplicates.

#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;


int main() {
	srand(time(0));
	int arr[100];
	int dup, n, random;
	cout << "=============Find the dups in an Array=============\n \n";


	cout << "Enter the size of the array: "; cin >> n;

	for (int i = 0; i < n; ++i) {
		random = rand() % 100 + 1;
		arr[i] = random;
		dup = arr[i];
		for (int j =0; j < i; ++j) {
			if (dup == arr[j]) {

				cout << dup << " ";

			}

		}
	}
}

[dutch]

Alternative version.

#include <iostream>
#include <random>
#include <algorithm>
using namespace std;

int main()
{
    int size = 0;
    cout << "size: ";
    cin >> size;
    if (size < 0 || size > 100)
    {
        cerr << "size must be from 0 to 100\n";
        return 1;
    }

    int nums[100];
    iota(nums, nums + 100, 1);
    shuffle(nums, nums + 100, default_random_engine(random_device{}()));

    int *a = new int[size];
    copy(nums, nums + size, a);
    
    for (int i = 0; i < size; ++i) cout << a[i] << ' ';
    cout << '\n';

    delete[] a;
}