0

I have an Arduino sketch which generates random numbers. I need to populate an array with 5 unique generated numbers. In order to do that inside the loop() I did the following:

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

number=mappedForNumber(50);

if (isUnique(number)==true){

 numbers[i]=number;

}

}

where number is a global int and numbers is an array, also declared globally at the top of the sketch

the function "isUnique" is as follows:

bool isUnique (int foo){

  bool pepe=true;

   int sizeOfNumbers=sizeof(numbers)/sizeof(numbers[0]);
    
    for (int i=0; i<sizeOfNumbers; i++){

      if (numbers[i]==foo){

        pepe=false;
        break;
      }

    }

  
  return pepe;
  }

When I check the results, it doesn't really work. If I generate say 30 arrays and I print them on the serial monitor, there is always some of the arrays with duplicated numbers.

I haven't coded for a long while and I'm super rusty. Any help would be appreciated. Thanks in advance!

Improve this question
6
  • In any random sequence, repeats are to be expected. If you want a sequence in which no digit appears more than once, then that's (a) not "random", and (b) something you have to tell your computer to do. Commented Jul 12, 2020 at 0:16
  • 1
    How often do you need to generate these random numbers? A common way to do this is to shuffle a deck of cards. If your cards are numbers from 1 to 50, and you shuffle your deck, and draw the top 5 cards, you have your five randomly selected unique numbers. Commented Jul 12, 2020 at 0:17
  • 1
    Is there an else in the first loop after if (isUnique(number))? Commented Jul 12, 2020 at 0:19
  • May be you better use "std::set" instead of array, which simplifies your job. Commented Jul 12, 2020 at 0:22
  • 1
    If the number is unique, you put it into the array. If it's not unique, you skip that index and leave that element of the array with its original value. What values are in the array initially? Some of them are still there afterwards. Commented Jul 12, 2020 at 0:35

2 Answers 2

Reset to default
2

You are using a for loop to populate your numbers array. This for loop will populate the selected index with a number if it is unique and not change the data inside the index if it isn't.
To fix your code:

int i = 0;
int number;
while(i < 5)
{
  number = mappedForNumber(50);
  if (isUnique(number) == true)
  {
    numbers[i]=number;
    i++;
  }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

That made the trick, apparently. I was only focused on the isUnique function and didn't occur it could be something else... thanks!
No problem. In my experience, intermittent code errors are rarely in a well reviewed function.
0

I would use a truncated Fisher-Yates algorithm to do this, assuming you need to select sets of five numbers often.

Given a deck of "cards" labeled from 1 to 50, you shuffle your deck, and draw the top five cards. Use them for whatever you want.

However, since you only ever need five, you just need to shuffle the "top five". This is the "truncated" part of Fisher-Yates. Iterate from the end of the deck down five cards and select five cards below that card (including that card) to swap with that one.

Then when you need five more random numbers, shuffle again.

// swaps two integers
void swap(int& a, int& b) {
  int temp = a;
  a = b;
  b = temp;
}

// returns a pointer to an array of ints randomly
// selected between consecutive values in [1, 50]
// num_ints_requested must be <= the size of the deck
const int* ShuffleRandomInts(int num_ints_requested) {
  constexpr int kNumInts = 50;
  static bool initialized = false;
  static int deck[kNumInts];

  if (!initialized) {
    for (int i = 0; i < kNumInts; ++i) {
      deck[i] = i + 1;
    }
    initialized = true;
  }

  // do the truncated shuffle
  for (int i = kNumInts - 1; i >= kNumInts - num_ints_requested; --i) {
    int j = random(0, i);
    swap(deck[i], deck[j]);
  }

  return &deck[kNumInts - num_ints_requested];
}

void loop() {
  // do work
  ...

  // get 5 random numbers between 1 and 50
  // and do something with them
  const int* numbers = ShuffleRandomInts(5);
  DoSomethingWithFive(numbers);

  // get 10 random numbers between 1 and 50
  // (this invalidates the previously selected numbers)
  numbers = ShuffleRandomInts(10);
  DoSomethingWithTen(numbers);

  // do other stuff
  ...
}

If you need to use one set of randomly selected numbers after generating another set of randomly selected numbers, then copy those numbers to a new array before reshuffling.

We return a pointer to const int so that you don't accidentally modify the deck after returning it. To extend the shuffled deck analogy, this would be like dealing five cards to somebody, and having them draw on the cards with a marker before handing them back to you to shuffle again. You don't want that, so you specify that you can't modify the numbers after returning them.

(If you need to modify them, you have to copy them to a non-const int or non-const int array).

Improve this answer

5 Comments

I think your code is inefficient. It never checks if num_ints_requested >= 50 (possible crash scenario). It recreates the deck on every call to the function. It modifies the order of the items num_ints_requested random items in the array, which means that very likely the first num_ints_requested items will not change for a small enough num_ints_requested. The previous order of the deck is not maintained, so there is no real use to it. It is way more operations per call to the function than the question. I don't think he wants or has access to rand(0, i) in an Arduino Sketch.
Good concerns, @Alek. Not every input needs to be validated. Sometimes its more appropriate to trust the programmer to use the function correctly. This check could be added easily if you like, and maybe return a nullptr as a fail condition. The deck and initialization variable are static, so the deck is only created once. I don't understand your point about a small request causing the numbers not to shuffle? The previous deck order is not maintained, correct. If the given order after a shuffle needs to be saved the user must copy the values somewhere.
How many more operations per call to the function exist than the question? It has complexity N in number of randomly selected values requested. OP is getting randomness somewhere. I made a typo on the correct function. Looks like Arduino uses random, not rand. Thanks for that. arduino.cc/reference/en/#functions
My mistake on that, didn't realize the components were static and you are just playing with the end of the array. Definitely interesting and quite efficient, granted not very thread safe.
I also like that your code always produces a result in O(num_ints_requested) number of operations with a one time overhead of O(kNumInts) even if num_ints_requested = kNumInts, which is not true for the original function.

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.