Need help with functions.

[hujiba]

I'm new at this so sorry if my vocabulary and stuff is totally off.
With the following syntax, my goal is to call the choose_number function, which, when given two arguments (integers), returns one of those two at random. I run it multiple times with a for loop but it doesn't seem to randomize. Maybe it randomizes the first time, but the other times the loop runs, the value remains the same. Maybe eliminate_door is only being defined once and then that same value is printed over and over again? If you could help me out that would be great.

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

using namespace std;

int choose_number(int option_a, int option_b)
	{
		srand(time(0));

		int random = rand() % 2;

		if (random == 0)
		{
			return option_a;
		}
		else
		{
			return option_b;
		}
	}

int main()
{
	srand(time(0));

	int door1 = 1;
	int door2 = 2;
	int door3 = 3;

	for (int i = 1; i <= 300; i++)
	{
		int eliminate_door = choose_number(door2, door3);
		cout << eliminate_door << endl;
	}

	system ("pause");
	return 0;
}

[arkoenig]

Remove the call to srand from the choose_number function.

As a general rule, you should call srand only once in your entire program.

[hujiba]

Ahh awesome man thanks. I didn't know that that could be a problem. Works perfect now thanks.

[ravenous]

Hi there, well done for getting your function to work as you wanted :o) Now that it's working you could do a little tidying, since choose_number is a little more verbose than it could be. In fact, the whole thing only needs one line:

int choose_number( int option_a, int option_b )
{
   return (rand() % 2 == 0) ? option_a : option_b;
}

The ternary operator ?: is very useful for this kind of situation and once you get used to it, I find that it makes code more readable :o)

If you want to go further, you could use a function template here, so that you can get a choose_number for double , or std::string or indeed any class that you might want to create yourself:

template< typename T >
inline T choose_item( const T& item_1, const T& item_2 )
{
   return (rand() % 2) ? item_1 : item_2;
}

With this template, now you get functions to do al these things for free:

class MyClass{
/* ... */
};

int main()
{
   srand(time(0));
   
   int door1 = 2;
   int door2 = 5;
   int eliminate_door = choose_item(door2, door3);
   std::cout << eliminate_door << std::endl;

   std::string someString( "Foo" );
   std::string otherString( "Bar" );
   std::cout << choose_item( someString, otherString ) << std::endl;

   MyClass A, B;
   MyClass C = choose_item( A, B );

   return 0;
}

I'd also be annoyed by that pesky srand() hanging about in main() too. You can get rid of this by encapsulating it with the function that uses it in a class, let's call it RandomChooser :

struct RandomChooser{
   RandomChooser(){   srand( 0 );  }
   RandomChooser( const RandomChooser& RC ){}
   template< typename T >
   inline T Choose( const T& item_1, const T& item_2 )
   {
      return (rand() % 2) ? item_1 : item_2;
   }
};

int main()
{
   RandomChooser myChooser;

   std::cout << myChooser.Choose( 1.0, 9.9 ) << std::endl;

   std::cout << myChooser.Choose( "Foo", "Bar" ) << std::endl;
}

The situation is slightly complicated because you only want one call to srand once, in the whole program, so if you declare another chooser somewhere then you'll reset the random number sequence for all the choosers.

Sorry about the slightly pointless and rambling nature of this reply, I just thought that you might like to know what some further possibilities for you function are :o)

[hujiba]

Oh wow thanks a lot! I'm actually taking an introductory programming course right now so I'm very much looking forward to learning this stuff.

[arkoenig]

@ravenous: Thanks for the excellent suggestions. I have just one tiny comment to add: Some people may prefer

return (rand() % 2)? item_2: item_1;

to

return (rand() % 2 == 0)? item_1: item_2;

Others may feel it's too clever. I'm mentioning it so that you can make up your own mind.

In a similar vein, the outer parentheses are unnecessary because of operator precedence, so you could write

return rand() % 2 ? item_2: item_1;

with the same effect.

Finally, because you're making a random selection, the order of item_1 and item_2 doesn't really matter.

[Fbody]

struct RandomChooser{
   RandomChooser(){   srand( 0 );  }
   RandomChooser( const RandomChooser& RC ){}
   template< typename T >
   inline T Choose( const T& item_1, const T& item_2 )
   {
      return (rand() % 2) ? item_1 : item_2;
   }
};

The situation is slightly complicated because you only want one call to srand once, in the whole program, so if you declare another chooser somewhere then you'll reset the random number sequence for all the choosers.

You could alleviate that situation by creating a "singleton like" class. Something like this:

class RandomChooser {
  private:
    static bool seeded; //a static check variable, there will only ever be 1 in existance
                        //and all instances of RandomChooser will use the same one
  public:
    RandomChooser() {
      if (!seeded) {    //see if the RNG has already been seeded
        srand((unsigned)time(0));
        seeded = true;
      } //end if
    }   //end default constructor

    RandomChooser (const RandomChooser &RC) {}  //copy constructor

    template <typename T>
    static T Choose (const T &item_1, const T &item_2) {
      return rand() % 2 ? item_1 : item_2;
    }
};

bool RandomChooser::seeded = false; //initialize the static member variable

int main() {
   RandomChooser myChooser;
 
   std::cout << myChooser.Choose( 1.0, 9.9 ) << std::endl;
 
   std::cout << myChooser.Choose( "Foo", "Bar" ) << std::endl;

   return 0;
}

This returns a template specialization error when I try to compile, but I think that's because VC++ doesn't do specializations very well (it doesn't even compile the original non-static code). It should give you the basic idea though.

[doug65536]

I have a suggestion to get better randomness from rand().

The high bits are the most random, the low bits are a lot less random. If you use modulus, you are effectively using the low bits - your random numbers won't be very random.

To get optimal randomness from rand(), you should use the high bits.

rand() returns a value between 0 and RAND_MAX. My suggestion is to use the following:

int randomNumber = rand() * range / RAND_MAX;

where randomNumber is the random number (obviously), range is the highest value you want (in your code, it's 1) and RAND_MAX is a constant defined in the standard library header file.

You might run into trouble if you are using a large number for 'range'. Since C99 defines rand() as returning an int, you can guarantee correct behavior by making the expression a little more robust:

int randomNumber = (int)(((long long)rand() * range + (RAND_MAX / 2)) / RAND_MAX);

Almost everybody uses rand() wrong. Your lessons probably are teaching you to use it wrong too. When the randomness is important, remember, use the high bits from rand(), not the low bits.