Function Issue

im not sure why char x is not displaying anything at the end.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
  
#include <iostream>
#include <cstdlib>

using namespace std;
   char value_assigner(char pos){
      
     char g;
     int e;
     srand(time(0));
     e=rand() % 300+1 ;
  
     if(e%2==0){
      g='O';
  
      }
      else if (e%2!=0){
      g='X';
      }
      g=pos;
      return pos;
   }


int main()
{
    char x;
    switch(0){
        case 0: value_assigner(x);
        break;
    }
    cout<<x;

    return 0;
}
Last edited on
x is uninitialized. It is not predictable what the content is. Most likely not a printable character.

Change line 27 to:

char x = '1';

and it will display 1.
Also, I note that you're not doing anything with the value that's returned by value_assigner().

Also, that's the point of assigning a value to g at line 14 or 18, when you immediately overwrite that value with a new one at line 20?

And whatever value you assign to g, you don't do anything with that value anyway.
im still not sure what im doing wrong. i tried changing it a little but it still doesnt output anything when i call cout at the end. it should be a random combination of 'XO'.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
#include <iostream>
#include <cstdlib>

using namespace std;
   char value_assigner(char g){
      
      
     int e;
     srand(time(0));
     e=rand() % 300+1 ;
  
     if(e%2==0){
      g='O';
  
      }
      else if (e%2!=0){
      g='X';
      }
        return g;
     
   }


int main()
{
    char x,y;
    for(int i =0; i <3; i++){
    switch(i){
        case 0: value_assigner(x);
        break;
        case 1: value_assigner(y);
        break;
    }
    }
    cout <<x<<y;
    

    return 0;
}
You are not assigning x or y! Just because you called your function "value_assigner" does not magically make that happen.

Line 5: When you pass in char, it is being copied by value, this this copy, g, is being assigned and returned.

Line 29 & 31: You pass in x and y, which are then copied. But the original x and y variables are left unchanged, and undefined.

Possible solution:
1
2
3
4
5
6
    switch(i){
        case 0: x = value_assigner(x);
        break;
        case 1: y = value_assigner(y);
        break;
    }
Last edited on
first, in value_assigner, the second if is redundant. If e is even, O, else X. Much simpler to write
//eliminate e, and after moving srand, the whole function becomes
g = 'X';
if ( (rand()%300+1) == 0)
g = 'O';
return g;

srand should only be called once in a program for normal uses. Call it in main.

switches should have a default case.

the main function is extra convoluted. If this is place-holder code for future work, ok, but if this is what you want, simply do this:

int main()
{
srand(time(0));
char x; //and honestly this can be removed as well, but I left it for now.
cout << value_assigner(x) << value_assigner(x);
return 0;
}


Finally, the actual bug and cause of your problems (the above is just cleanup, you are doing way too much work for what needs doing across the board. Keep it simple where you can!)
change it to
x = value_assigner(x);
y = value_assigner(y);

OR if you want it to work the way you have it, do this INSTEAD
void value_assigner(char &g)
Last edited on
ok i understand that its really bad looking and i will clean it up once i get the correct output. but the problem im still getting is the output is always XX or OO. its never a combination. i can have XX or OO but it shouldnt be the output everytime.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
lude <cstdlib>

using namespace std;
   char value_assigner(char *g){
      
      
     int e;
     srand(time(0));
     e=rand() % 300+1 ;
  
     if(e%2==0){
      *g='O';
  
      }
      else if (e%2!=0){
      *g='X';
      }
        return *g;
     
   }


int main()
{
    char x,y;
    for(int i =0; i <3; i++){
    switch(i){
        case 0: value_assigner(&x);
        break;
        case 1: value_assigner(&y);
        break;
    }
    }
    cout <<x<<y;
    

    return 0;
}
Read jonnin's post again. Specifically, the line talking about srand.
Last edited on
thank you guys so much.
if its cool can someone explain why srand not being in main was the problem?
srand is what seeds rand. srand(time(0)) seeds rand with the current second in time. If you call srand twice within the same second, you will be reseeding it with the same value, and it will cause rand() to generate the same output each time.
Looking at the latest code you've posted - there's no point at all passing g as an argument to the function. You're returning the value your function generates anyway, so why do you also need to take an argument for the function to modify?
Last edited on
there is a c++ newer random tool, but some more info..

srand basically sets a starting point. using time randomizes this so its not the same every time you run your program.

if you just did this
srand(3);
and printed the first 10 random values from rand as a program
it would print the same 10 values every time forever.
that can be useful when debugging or if you need a stream that is the same every time. It is not terribly random when you need actual random values, though, so after debugging you set it to time(0).

calling it twice in a row very quickly is the same as calling it with srand(3); srand(3); because time(0) changes slowly relative to execution speeds. You reset the stream and get the same number twice every time.

but that isnt the bug per-se. The bug, as I said above, was not assigning your values at all. The srand bug just makes you get 2 of the same value every time you run.

The pointer thing is, like the rest of the code, the most complicated solution you could find.

look.

void value_assigner(char &g)
{
g = 'X';
if(rand()%2 == 0) //the 300+1 is not making it any more random or doing anything at all.
g = 'O';
}

main
value_assigner(y);
value_assigner(x);

that will work.

Last edited on
Topic archived. No new replies allowed.