2

Hey I am trying to write some numbers to a file, but when I open the file it is empty. Can you help me out here? Thanks.

/** main function **/
int main(){

    /** variables **/
    RandGen* random_generator = new RandGen;
    int random_numbers; 
    string file_name;   

    /** ask user for quantity of random number to produce **/
    cout << "How many random number would you like to create?" << endl;
    cin >> random_numbers;

    /** ask user for the name of the file to store the numbers **/
    cout << "Enter name of file to store random number" << endl;
    cin >> file_name;

    /** now create array to store the number **/
    int random_array [random_numbers];

    /** file the array with random integers **/
    for(int i=0; i<random_numbers; i++){
        random_array[i] = random_generator -> randInt(-20, 20);
        cout << random_array[i] << endl;
    }

    /** open file and write contents of random array **/
    const char* file = file_name.c_str();
    ofstream File(file);

    /** write contents to the file **/
    for(int i=0; i<random_numbers; i++){
        File << random_array[i] << endl;
    }

    /** close the file **/
    File.close();   

    return 0;
    /** END OF PROGRAM **/
}
user69514
  • 26,935
  • 59
  • 154
  • 188
  • 1
    Just a glance shows that this code won't compile - the "random_array" array is declared with a non-constant variable for its length. You should post your actual code, I suspect you've over simplified – Terry Mahaffey Apr 06 '10 at 02:28
  • @Terry: it's a nonstandard extension. – Potatoswatter Apr 06 '10 at 02:30
  • @Potatocorn: Really? Good heavens, on which platform? – Cameron Apr 06 '10 at 02:31
  • @Terry, `gcc` supports that in C++ as a C99 extension. – vladr Apr 06 '10 at 02:33
  • @Cameron: It's standard C99. g++ allows this extension from the gcc portion of the compiler. – GManNickG Apr 06 '10 at 02:35
  • For myself, I would consider almost all of your comments necessary. The program describes itself fairly well. – dreamlax Apr 06 '10 at 02:36
  • 1
    In any case, is there a reason `random_generator` is dynamically allocated? For example of why automatic allocation is always preferred, you're leaking it. Also, wait until you need variables before you declare them; your code is hard to read otherwise. Also you named your file variable `File`, which doesn't fit in with your other variables, which is strange. It could just be `ofstream file(file_name.c_str());`. Additionally, there is no need to manually close the file, it's done automatically. Lastly, your error is here: `int random_array [random_numbers];`. Needs to be a `std::vector`. – GManNickG Apr 06 '10 at 02:38
  • 1
    I find it's always a good idea to explicitly close files and release memory even immediately before program exit, simply because otherwise, if that code is ever moved around, it's easy to forget the cleanup. – Cameron Apr 06 '10 at 02:54
  • @Cameron: What do you mean? Memory should be in a class that releases it automatically, like `vector`. (Nothing to worry about here then.) Likewise, file closes automatically. – GManNickG Apr 06 '10 at 03:05

3 Answers3

4

You can't declare an array of integers with a size known only at run-time on the stack. You can declare such an array on the heap however:

int *random_array = new int[random_numbers];

Don't forget to add delete [] random_array; at the end of main() (and delete random_generator; too) to deallocate the memory that you allocated using new. This memory is automatically freed when your program exits, but it's a good idea to release it anyway (if your program ever grows, it's easy to forget to add it in later).

Apart from that, your code looks fine.

GManNickG
  • 494,350
  • 52
  • 494
  • 543
Cameron
  • 96,106
  • 25
  • 196
  • 225
  • Might I suggest a std::vector instead? :) – Billy ONeal Apr 06 '10 at 02:39
  • 1
    -1, sorry. Use a `std::vector`, there's no reason to do it by hand. – GManNickG Apr 06 '10 at 02:39
  • 1
    @GMan: I agree that there's many better ways of writing the code (`std::vector` is a good idea)... but it is a sufficiently simple example that manual memory management is not likely to become an issue. I was trying to keep in line with the original code (which does not look like it was written by someone quite prepared to delve into the STL just yet). – Cameron Apr 06 '10 at 02:51
  • @GMan not sure I agree, nothing wrong with doing it by hand especially for a simple example like this one – Sam Post Apr 06 '10 at 02:56
  • @Cameron: We disagree then. I think people should use the standard library *first*, then learn how it works. Teaching people to use manual memory management isn't necessary until they need to know how `vector` works. Until they know memory management and all it's caveats, suggesting it will only help them make messy, unsafe, and poor code. I'll remove the -1 for good intentions though. – GManNickG Apr 06 '10 at 02:56
  • @Sam: Like I said to Cameron: why *not* use a vector? It's not like you're bringing in some behemoth, you're using standard C++! I would slap (virtually) any programmer that used manual memory management over vector in a real project. Do you suggest people write their own floating point class wrapped around raw bits instead of using float? Then why are you suggesting people do things the hard way in the memory department? There's everything wrong with it unless for the specific case of learning how vector works, which isn't the case here. `vector` is easier, safer, and cleaner. – GManNickG Apr 06 '10 at 02:58
  • @GMan: Interesting, we do disagree. I think it's a good idea to learn how things work first, then learn (and use) the abstractions (within reason of course -- learning how everything worked from the ground up would leave no time for doing anything useful). There's no single "correct" level of abstraction, or at least not one that everyone could agree on ;-) And yes, `std::vector` is easier, but it comes with a learning curve (I remember struggling with the funny template syntax at first). – Cameron Apr 06 '10 at 03:01
  • @Cameron: But abstraction in general is correct. I think it's good to learn how things work, but not unnecessarily. Teach people to write good C++ code (using `new[]` is *not* good C++ code, ever); when the time comes, they'll learn how it works. Black boxes are ok. – GManNickG Apr 06 '10 at 03:04
  • If you are going to allocate your own array, at least use `boost::scoped_array` or a similar RAII container to ensure exception safety. – James McNellis Apr 06 '10 at 04:14
0

If I just fill in your RandGen class to call rand, the program works fine on Mac OS X 10.6.

How many random number would you like to create?
10
Enter name of file to store random number
nums
55
25
44
56
56
53
20
29
54
57
Shadow:code dkrauss$ cat nums
55
25
44
56
56
53
20
29
54
57

Moreover, I see no reason for it not to work on GCC. What version and platform are you running on? Can you provide complete source?

Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
0

There's no need to loop twice or keep an array or vector.

const char* file = file_name.c_str();
ofstream File(file);

for(int i=0; i<random_numbers; i++){
    int this_random_int = random_generator -> randInt(-20, 20);
    cout << this_random_int << endl;
    File << this_random_int << endl;
}

File.close();
dreamlax
  • 93,976
  • 29
  • 161
  • 209