2
\$\begingroup\$

Can my rand_int function be improved?

#include<stdio.h>
#include<stdlib.h>
#include<time.h>

int rand_int(int a,int b)
{
    //The basic function here is like (rand() % range)+ Lower_bound

    srand(time(0));
    if (a > b)
        return((rand() % (a-b+1)) + b);
    else if (b > a)
        return((rand() % (b-a+1)) + a);
    else
        return a;
}

int main()
{
    int a,b;

    printf("\nEnter the range in which the a random no. is required: ");
    scanf("%d %d", &a, &b);

    printf("%d\n", rand_int(a,b));
}
\$\endgroup\$
5
  • \$\begingroup\$ I think that your main problem is in the concept, rand() is probably the most unreliable prng, but this is not the real downside, the real problem is that rand() in just defined as a generic pseudo-random number generator that returns a value between 0 and RAND_MAX, no words about the real algorithm that rand() uses, so you can work on rand() as much as you want but you will never get a consistent behaviour or even a predictable one. You also should describe what kind of PRNG you are looking for in a question like this one. \$\endgroup\$ Commented Jul 29, 2013 at 8:52
  • \$\begingroup\$ @user2485710 In a real-life problem when I want randomness the pnrg used in rand() would matter but here I was just looking for how to use rand() properly. Mainly for timing codes. \$\endgroup\$ Commented Jul 29, 2013 at 8:56
  • \$\begingroup\$ yes, infact I just avoided to comment on the "unreliable" side of rand(), my point is since rand() is not defined by standard and is implementation-dependant, you are basically taking an educated guess no matter what you are doing with rand(), no matter what your goals or requirements are. \$\endgroup\$ Commented Jul 29, 2013 at 9:04
  • \$\begingroup\$ @user2485710 So is it always better to write my own prng? \$\endgroup\$ Commented Jul 29, 2013 at 9:06
  • \$\begingroup\$ Considering that your language of choice is C, yes, C doesn't really offer a good PRNG out of the box, but there are also a lot of PRNGs that are extremely easy to implement ( especially if you don't set the bar really high and you can live without a super-complex PRNG for a banking system ) en.wikipedia.org/wiki/Xorshift \$\endgroup\$ Commented Jul 29, 2013 at 9:09

1 Answer 1

3
\$\begingroup\$

First of all. Move srand() out of the function call. For now, it is not random at all if you manage to execute function twice in the same second. You will reset the random seed to the same value with time(0) as it normally has a 1s resolution.

Then I would suggest to do a conditional swap, and then perform "generate random" thing. This way you will split it into "preparation of arguments" and the "actual work parts".

You will have one exit point, which is a bit more readable and AFAIK tend to be a bit more optimizer friendly. Also, modern processors don't like too many ifs, especially unpredictable. So I would further suggest refactoring the "job" part into a separate function int rand_hi_lo(int upper, int lower) maybe even inlined. You could skip the branch at all then in certain cases.

#include<stdio.h>
#include<stdlib.h>
#include<time.h>

void swap_int(int* a, int* b){ int tmp = *a; *a=*b; *b=tmp; }

inline int rand_int_hi_lo(int upper, int lower){
  return((rand() % (upper-lower+1)) + lower);
}

int rand_int(int a,int b)
{
    if (b > a) swap_int(&a,&b);
    return rand_int_hi_lo(a,b);
}

int main()
{
    int a,b;
    srand(time(0));
    printf("\nEnter the range in which the a random no. is required: ");
    scanf("%d %d", &a, &b);
    printf("%d\n", rand_int(a,b));
}

It might be even possible to skip the conditional at all. But I am not sure about the negative modulo arithmetics. I think it is possible. Also my c is rusty, so I hope I wrote swap correctly.

EDIT: I also skipped the a==b. I don't think it is worth to put there at all unless you are sure it happens more than rarely. If necessary I would put if (a==b) return a; it before the swap_int.

\$\endgroup\$
7
  • \$\begingroup\$ Your swap is correct but can you elaborate the part about inline? I have heard that it is good but I haven't used it before. Also didn't get how avoiding many ifs leads to a separate function \$\endgroup\$ Commented Jul 29, 2013 at 9:04
  • \$\begingroup\$ It used to indicate that the function body might be embedded into the caller's body, when generating assembler, to avoid unnecessary jumps. It used to be a performance option. Now it has mostly semantical meaning, as the compilers are smart enough to figure out what to inline. They were also free to ignore it. Regarding ifs. It is the opposite. Refactoring helps to provide an additional function that avoids conditional, which might be important perfomnace-wise; related to "branch predictions" and "code path execution" stuff. \$\endgroup\$
    – luk32
    Commented Jul 29, 2013 at 9:17
  • \$\begingroup\$ In this example how does using rand_int_hi_lo function avoid the if? You can simply put that into the rand_int function and it would be the same \$\endgroup\$ Commented Jul 29, 2013 at 13:13
  • \$\begingroup\$ It allows you to skip it, when you know the order of operands is right. \$\endgroup\$
    – luk32
    Commented Jul 29, 2013 at 13:15
  • \$\begingroup\$ So you are saying that in the function rand_int the if wouldn't be executed because of the use of the function call to rand_int_hi_lo? How is that possible? The if is before that function call. \$\endgroup\$ Commented Jul 29, 2013 at 13:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.