3

I am learning C++ and I can't seem to find an answer to my issue. When I run my code, I don't get any compiler errors, but I when I call a function "getVin()" (supposed to generate a random number with the use of "generate()" function) it doesn't do so. It outputs a zero. Here is my class (from header file):

class Vehicle {
public:
    Vehicle();
    static int generate();
    const int getVin () { return m_vin; }

protected:
    float m_lla[3];
    const int m_vin = s_idgen;

private:
    static int s_idgen;
};

And definition (from source file):

int Vehicle::s_idgen = generate();

Vehicle::Vehicle() {
    m_lla[3] = 0;
}

int Vehicle::generate() {
    srand((int)time(0));
    return (rand() % 10000) + 1;
}

Any advice would be helpful, thank you!

gsamaras
  • 71,951
  • 46
  • 188
  • 305
LCKDWN
  • 31
  • 5
  • 2
    You should call `srand` only *once*. For example, the `time` function usually returns the time in *seconds*, meaning if you call your `generate` function multiple times within a single second then you will reset the seed to the same value and get the same "random" number. Also, C++ have much better [pseudo-random generation facilities](http://en.cppreference.com/w/cpp/numeric/random) than plain `srand` and `rand`, and I recommend you use those instead. – Some programmer dude Oct 11 '17 at 07:32
  • Think about this: when does this happen? `const int m_vin = s_idgen;` And when does `s_idgen` get set? – juanchopanza Oct 11 '17 at 07:38
  • 2
    Don't think the question is a duplicated of the suggested question: OP problem is not related to repeated call to srand, but to order of initialization of static variable, as per @Serge Ballesta anwer. – Gian Paolo Oct 11 '17 at 08:06

2 Answers2

1

I could partially reproduce, so I assume that you were bitten by the static initialization fiasco. I have just added:

Vehicle sveh; // static scoped

immediately after Vehicle declaration and before any method or static field definition, and then

int main() {
    Vehicle veh;
    std::cout << veh.getVin() << std::endl;
    std::cout << sveh.getVin() << std::endl;
    return 0;
}

Output is:

1915
0

That means that an automatic Vehicle correctly uses the random value (random for a run, but common to all instances...), while the static one was initialized before the static field initialization.

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
0

In the header, you do:

protected:
  const int m_vin = s_idgen;

while in your source file, you do:

int Vehicle::s_idgen = generate();

When the initialization of m_vin happens, what is the value of s_idgen? It's not yet set by generate(). Try to print it out, to see what I mean.

Try returning s_idgen from your function directly.


PS: Consider using <random> instead of C-traditional functions.

gsamaras
  • 71,951
  • 46
  • 188
  • 305