2

What is the problem with this code?

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

void createNumb(int RandomNumber) {

    srand(time(NULL));

    int a, b, c, d;

    a = rand() % 10 + 0;
    b = rand() % 10 + 0;
    c = rand() % 10 + 0;
    d = rand() % 10 + 0;

    while (b == a) {
        b = rand() % 10 + 0;        
    }

    while (c == b || c == a) {
        c = rand() % 10 + 0;     
    }

    while (d == c || d == b || d == a) {
        d = rand() % 10 + 0;
    }

    RandomNumber = ("%d%d%d%d", a * 1000 + b * 100 + c * 10 + d * 1);   
}

int main() {   
    int x;
    createNumb(x);
    printf("%d", x);
}

When I run this program, it always give me the result 0.

I want it to print a random 4-digit number (each digit must be unique). What is the problem?

Gilles 'SO- stop being evil'
  • 104,111
  • 38
  • 209
  • 254
blackwater7
  • 107
  • 1
  • 9

2 Answers2

3
  • Your createNumb function returns void instead of an int.
  • You should use srand() only once, so it’s better move it to your main.

Your createNumb function should return a random integer, like this:

int createNumb(void) {
    /*
    ** Simplified for readability.
    ** Here you would use your function that generates 4 different digits
    */
    return rand();
}

int main(void) {
    srand(time(NULL))(
    int x = createNumb();
    printf("%d" , x);
}

Or, if you want createNumb to fill an existing integer, use a pointer:

void createNumb(int *randomNumber) {
    /*
    ** Simplified for readability.
    ** Here you would use your function that generates 4 different digits
    */
    *randomNumber = rand();
}

int main(void) {
    int x;
    srand(time(NULL))
    createNumb(&x); // Passing a pointer to x
    printf("%d" , x);
}
Ronan Boiteau
  • 9,608
  • 6
  • 34
  • 56
  • 1
    I'm afraid you removed the constraint *each digit must be unique*. – chqrlie Mar 11 '18 at 08:33
  • can i ask you one more thing? lets say i have this array in the createNumb function : int ArrayOfNumbers[4] = {a,b,c,d}; And, in main function, I want to check if user input same with this array. Or ,For example, I want to check if 5 is higher than c. How do I do that? When I try to compare them, it says each undeclared identifier is reported only once for each function it appears in – blackwater7 Mar 11 '18 at 09:04
  • @blackwater7 You should open a new question with your code and the exact error. It would be easier for us to help you! – Ronan Boiteau Mar 11 '18 at 09:11
  • @Ronan Boiteau I know I could but it will be like I'm spamming all over the place... if it will be okay, I can do that^^ – blackwater7 Mar 11 '18 at 09:12
  • 1
    @blackwater7 You could also edit your question if the problem is related. But from what you said it seems unrelated so I think a new question is better. People who get the same error as you could find your question and use it solve their problem :) – Ronan Boiteau Mar 11 '18 at 09:16
  • @RonanBoiteau It allows me to post only once in every 90 minutes. Can you please check this pic, its my question... Its very easy. https://i.gyazo.com/f9dfaf592b705d1388bcb035764c4783.png – blackwater7 Mar 11 '18 at 09:24
  • @blackwater7 In C you can't access a variable from another function like you did. You need to return the value. Here's how: at the end of your `sampleXX` function, you need to `return x;` and in your `main` you need to get that returned value like this: `int x = sampleXX();`. Then you'll be able to print `x`'s value. – Ronan Boiteau Mar 11 '18 at 09:40
  • @RonanBoiteau Thanks for the reply. But there are some problems. Firstly, like I said I can only create a new thread after 90 minutes. I can't wait that much... And then, I know I can do it. But in my real homework, its already returning another value. I would text here the code, but it looks so complicated in reply section. This is why I take images. For example look at this pic: https://i.gyazo.com/99df1f109d5e2518daf13cdf03727cbf.png – blackwater7 Mar 11 '18 at 09:54
2

There are some problems in your C code, coming from a different language:

  • use return to return a value from a function and specify the return type before the function name`.
  • our attempt at formating the number is incorrect and unneeded. Incidentally, RandomNumber = ("%d%d%d%d", a * 1000 + b * 100 + c * 10 + d * 1); uses the operator , that ignores its left operand and evaluates to the right operand, whose value is stored into a local variable with automatic storage duration, and is lost upon function exit. Just use return a * 1000 + b * 100 + c * 10 + d * 1;.
  • to compute a 4 digit pseudo-random value, you could just use rand() % 10000, but an approach such as your may be needed for more digits or for specific constraints such as unique digits.
  • to make your results more random, use a faster changing source for srand() such as clock() and initialize the pseudo-random number generator just once int the main() function.
  • you could use the printf format "%04d" to make display an initial 0 if the result is less than 1000.

Here is a modified version:

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

int createNumb(void) {
    int a, b, c, d;

    a = rand() % 10;
    b = rand() % 10;
    c = rand() % 10;
    d = rand() % 10;

    while (b == a) {
        b = rand() % 10;        
    }

    while (c == b || c == a) {
        c = rand() % 10;     
    }

    while (d == c || d == b || d == a) {
        d = rand() % 10;
    }

    return a * 1000 + b * 100 + c * 10 + d;   
}

int main() {
    int x;
    srand(clock());
    x = createNumb();
    printf("%04d\n", x);
    return 0;
}

Note that you can avoid the loops in creatNumb(). Here is a modified version that takes an argument from the command line to produce multiple results.

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

int createNumb(void) {
    int a, b, c, d, bb, cc, dd;

    a = rand() % 10;
    b = rand() % 9;
    c = rand() % 8;
    d = rand() % 7;

    bb = (b >= a);
    cc = (c >= a) + (c >= b);
    cd = (d >= a) + (d >= b) + (d >= c);

    return a * 1000 + (b + bb) * 100 + (c + cc) * 10 + (d + dd);
}

int main(int argc, char *argv[]) {
    int x, n;
    srand(clock());
    n = (argc > 1) ? strtol(argv[1], NULL, 0) : 1;
    while (n-- > 0) {
        x = createNumb();
        printf("%04d\n", x);
    }
    return 0;
}

Boolean operators evaluate to 1 when they are true and 0 otherwise. bb = (b >= a); is a short notation for:

bb = 0;
if (b >= a)
    bb = 1;

If you want to compute an array of 4 numbers with the given constraint, you should pass a pointer to the destination array as an argument to creatNumb(). Here is a modified version with this approach:

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

void createNumb(int *dest) {
    int a, b, c, d, bb, cc, dd;

    a = rand() % 10;
    b = rand() % 9;
    c = rand() % 8;
    d = rand() % 7;

    bb = (b >= a);
    cc = (c >= a) + (c >= b);
    cd = (d >= a) + (d >= b) + (d >= c);

    dest[0] = a;
    dest[1] = b + bb;
    dest[2] = c + cc;
    dest[3] = d + dd;
}

int main(int argc, char *argv[]) {
    int array[4];
    srand(clock());
    createNumb(array);
    printf("%d%d%d%d\n", array[0], array[1], array[2], array[3]);
    return 0;
}

Note that passing an array as a argument to a function actually passes a pointer to its first element: createNumb(array); is equivalent to createNumb(&array[0]);. This process is referred to as arrays decay into pointers in most expression contexts, it is somewhat confusing for beginners but allows for efficient argument passing.

0x51ba
  • 463
  • 3
  • 12
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • What’s wrong with OPs void function other then it should have used a pointer and in main the address operator for X? Answer of @ronan is almost OK, because it is following the OPs requirement and this one not. – Michi Mar 11 '18 at 08:37
  • 2
    The only reason the OP seems to want to use a pointer is lack of experience in C. Introducing pointers is counterproductive at this stage. `int createNumb(void)` is a much more useful API than `void createNumb(int *)` which requires extra variables and whose result cannot be passed directly as an argument to another function. – chqrlie Mar 11 '18 at 08:45
  • I already explained in my comment that „THE ANSWER IS ALMOST OK“. My comment was only about the return part and not about the content of the createNum() – Michi Mar 11 '18 at 08:50
  • 1
    @user3386109: oops! not so simple after all. – chqrlie Mar 11 '18 at 08:57
  • can i ask you one more thing? lets say i have this array in the createNumb function : int ArrayOfNumbers[4] = {a,b,c,d}; And, in main function, I want to check if user input same with this array. or, For example, I want to check if 5 is higher than c. How do I do that? When I try to compare them, it says each undeclared identifier is reported only once for each function it appears in – blackwater7 Mar 11 '18 at 09:04
  • @user3386109: actually not so difficult, but Sunday mornings are slow ;-) – chqrlie Mar 11 '18 at 09:39
  • @ JonathanLeffler: I fixed the loopless version: it produces 5040 different results with a distribution that is very similar to that of `rand() % 10000`. – chqrlie Mar 11 '18 at 09:45
  • Interesting; I was getting quite skewed distribution results from my experiments with the main code, and my own variant of the loopless code. I'll play later with your new loopless code, and maybe try to work out whether the results I'm getting on the distributions are reasonable or not. They are nowhere near as uniform as I'd expect — which leaves me wondering whether it's my expectations that are faulty or my code (and your code, and ...). I presume it's my expectations that are bogus. I agree with 5040 different numbers with "4-unique digits including leading zero". – Jonathan Leffler Mar 11 '18 at 10:13
  • @JonathanLeffler: another thing to consider is the potential lack of uniformity of `rand() % 10`. A better way to get a pseudo-random number in a range `0..9` is `(int)((double)rand() / RAND_MAX * 10)` – chqrlie Mar 11 '18 at 10:51
  • 1
    Regarding bias in output from `rand() % n`: with RAND_MAX big enough, the bias is fairly small. When RAND_MAX was 32767 (which is the minimum allowed by the C standard), there could be a major skew, especially for a larger modulo value, but for 31-bit RAND_MAX, that is usually almost negligible, especially if the number is fairly small. If you're worried, you can use `int random_modulo(int n) { int max_ok = RAND_MAX - RAND_MAX % n; int value = rand(); while (value >= max_ok) value = rand(); return value % n; }` to generate an unbiassed value, without resorting to floating point arithmetic. – Jonathan Leffler Mar 12 '18 at 00:24
  • 1
    See also [Is this C implementation of the Fisher-Yates shuffle correct?](https://stackoverflow.com/questions/3343797/is-this-c-implementation-of-fisher-yates-shuffle-correct) – Jonathan Leffler Mar 12 '18 at 00:26
  • Still not seeing it. If `a=7`, `b=6`, `c=6`, and `d=1`, then `bb=0`, `cc=1`, and `dd=0` which results in `7671`. – user3386109 Mar 12 '18 at 06:26