0

I am running a code for finding repeating array elements.

I am doing it using 2 functions, however when I run the code my application immedietaly crashes despite assigning it to random numbers from 1 to 99.

Here is the code. Thank you..

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

int UniqueArray(int arr[], int notunique);

void printarray(int arr[]);

int main() {
  int arr[20];
  int dup = 0;
  printarray(arr);
  for (int i = 0; i < 20; ++i) {
    UniqueArray(arr, dup);
  }
}

int UniqueArray(int arr[], int notunique) {
  notunique = 0;

  int i, j;
  int size = sizeof(arr) / sizeof(arr[0]);
  for (i = 0; i < size; i++) {
    for (j = i + 1; j < size; j++) {
      if (arr[i] == arr[j]) {
        notunique++;
        cout << "Array has duplicates: " << arr[i] << " ";
      }
    }
  }
  return notunique;
  cout << "There were  " << notunique << " Repeated elements";
}

void printarray(int arr[]) {
  int size = sizeof(arr) / sizeof(arr[0]);
  srand(time(0));
  arr[20] = rand () % +100;
  for (int i = 0; i < 20; ++i) {
    cout << arr[i] << " ";
  }
}
cigien
  • 57,834
  • 11
  • 73
  • 112
Jos
  • 65
  • 7
  • 1
    `sizeof(arr) / sizeof(arr[0]);` does not do what you think it does: https://stackoverflow.com/questions/8269048/length-of-array-in-function-argument. But `arr[20] = rand () % +100;` is out of bounds anway, valid indices in `int arr[20];` are `0...19`. – Lukas-T Apr 28 '20 at 20:41
  • Thank your sir, I understand now. – Jos Apr 28 '20 at 21:05
  • This `int UniqueArray(int arr[], int notunique)` is *exactly* the same as `int UniqueArray(int* arr, int notunique)`. That's why the `sizeof` will not work -- you are getting the `sizeof(int *) / sizeof(int)`. Either use `std::array`, or pass the number of elements as a separate parameter, – PaulMcKenzie Apr 28 '20 at 21:09

1 Answers1

2

This line:

arr[20] = rand () % +100; 

does not fill an array of size 10 with random values. It indexes the 20th position, which is UB.

You could fill the array with random numbers, using std::generate, like this:

std::generate(arr, arr + 20, [] { return rand() % 100; });

Also, when finding the size of the array, you'll need to deduce the size:

template <size_t N>
void printarray(int (&arr)[N]) {
  // ... use N which is the size of arr

or even better, use std::array, which does this for you.

Some minor issues:

Don't use using namespace std;.

In this snippet:

return notunique;
cout << "There were  " << notunique << " Repeated elements";

the statement after the return will never get executed.

In this line:

arr[20] = rand () % +100; 

you don't need the + operator.

cigien
  • 57,834
  • 11
  • 73
  • 112
  • Hello, thank you for the answer, for std::generate does it require a special library? because I wrote it similarly and it doesn't recognize generate in my compiler. I am using Visual Studio 2019. I also see a return statement but my function is void. – Jos Apr 28 '20 at 20:53
  • It needs the header. – cigien Apr 28 '20 at 20:53
  • Alright I added the algorithm header, but when I run my function I don't get anything printed on screen. – Jos Apr 28 '20 at 20:55
  • Ok for the sizeof command when I add call by & to arr it says "dividing sizeof pointer by another value" I think this is the issue – Jos Apr 28 '20 at 21:00
  • How about dropping arrays altogether and just use `std::array`? Then you don't have to do all of that `sizeof` stuff, as `size()` gives you the number of elements. The `sizeof` cannot work here anyway. – PaulMcKenzie Apr 28 '20 at 21:05
  • the code works, but doesn't print if the array is duplicated or not. – Jos Apr 28 '20 at 21:21
  • @PaulMcKenzie I appreciate the suggestion, but I am not used to doing std::array. I am quite used to using namespace std; – Jos Apr 28 '20 at 21:22
  • @JohnnyJoestar -- Well, your function can never work using `int arr[]` as a parameter. The reason is as stated in the main comment -- that is not an array, it is just a pointer. So you've lost all size information, and no `sizeof` tricks will help you. So either use `std::array`, or pass the size as argument, or pass the array by reference. – PaulMcKenzie Apr 28 '20 at 21:24
  • As the answer says, you are printing that information *after* returning, which won't work. Swap those statements around. – cigien Apr 28 '20 at 21:26
  • Ok honestly speaking, I am not sure what pass size as argument or how to use std::array I am a beginner, and I personally don't understand arrays 100% – Jos Apr 28 '20 at 21:29
  • @JohnnyJoestar The bottom line is that when you pass an array to a function like that you lose information about the size of the array. There is no way to determine the size of the array inside of the function. One way to solve this is to pass the size of the array as a separate argument in the function. You would have the array as one argument and the size as another argument. – eesiraed Apr 28 '20 at 22:32
  • Thank you all I fixed the code already, I just removed the sizeof and arrays and used std::array, thank you again – Jos Apr 28 '20 at 22:42