0

So my program is supposed to generate 30 random numbers, put them in a file, then save those numbers in an array, but when I try to print out the numbers in a file they are not the numbers that are in the file but instead are some random huge numbers.

#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <fstream>
using namespace std;

void sortFile() {
    
    fstream f;
    int numbers[30];
    int n;
    
    f.open("f.txt");
    
    
    
    srand(time(NULL));
    for(int i; i < 30; i++) {
        f << rand() % 30 + (-9) << endl;
        
    }
    
    while(!f.eof()) {
        
        f >> numbers[n];
        n++;
    }
        
    f.close();
    
    for(int i; i<30;i++) {
        cout << numbers[i] << endl;
    }
        

}


int main() {
    
    sortFile();
    
}
vakru
  • 25
  • 2
  • 1
    You're not initializing `n`, which means your second loop likely overflows the `numbers` array and the third loop outputs garbage (because `numbers` is also uninitialized). Also you're not initializing `i` in both first and third loops, which means you may have basically any number of iterations. – Andrey Semashev Jun 14 '22 at 13:07
  • Mandatory reading: [Why is `iostream::eof()` inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – Ted Lyngmo Jun 14 '22 at 13:08

1 Answers1

-1

You should close the file after writing, and the open the file again when you start reading. You should also fix the while loop while (!f.eof()) is wrong. And you forgot to initialise n (as pointed out in comments above). Here's some fixed code

f.open("f.txt");                  // open file for writing
srand(time(NULL));
for(int i; i < 30; i++) {
    f << rand() % 30 + (-9) << endl;
    
}
f.close();
f.open("f.txt", ios_base::in);    // open file for reading
n = 0;
while(f >> numbers[n]) {
    n++;
}
f.close();

You have to be careful when switching between reading and writing. In your code you assumed that the file magically rewinds to the beginning when you change from writing to reading, but nothing happens by magic. Closing and reopening the file is one way to rewind the file. Another (perhaps better) way would be to use two separate file variables for reading and writing.

john
  • 85,011
  • 4
  • 57
  • 81
  • `while(f >> numbers[n]) {` - risky. What if someone put an extra number in the file when you looked the other way? :) – Ted Lyngmo Jun 14 '22 at 13:09
  • @TedLyngmo Even if there are exactly 30 elements in the file, this results in UB, at least [according to cppreference.com](https://en.cppreference.com/w/cpp/io/basic_istream/operator_gtgt): *"If extraction fails (e.g. if a letter was entered where a digit is expected), **zero is written to value** and failbit is set."* – fabian Jun 14 '22 at 13:21
  • I think this answer is incorrect. There are separate reading and writing positions for a stream. Writing to the stream does not modify the reading position, so you can write some data and then immediately read it back. [Here](https://gcc.godbolt.org/z/fq4sxjPax)'s an example with a string stream, it should work the same with files. – Andrey Semashev Jun 14 '22 at 13:21
  • @AndreySemashev You should try it with a file stream. It doesn't work like that on my system. I believe that the standard library allows for separate positions but doesn't require it. – john Jun 14 '22 at 14:03
  • Ah, I stand corrected. There is a single position for file streams. [Here](http://eel.is/c++draft/file.streams#filebuf.general-3.3)'s the relevant standard clause. – Andrey Semashev Jun 14 '22 at 14:46
  • @fabian Do you mean that failing to read an element will leave the rest of the elements in `numbers` uninitialized and therefore cause UB when printing them later? If so, I agree. One should check that it succeeds and also not read more than the array has room for, [example](https://godbolt.org/z/oMoox5vvY) – Ted Lyngmo Jun 14 '22 at 15:26
  • @TedLyngmo No, I mean that after 30 successful `operator>>` calls the extraction will be called with `numbers[30]` as "out param". Since there are only 30 elements in the `numbers` array, any write access to this out param results in undefined behaviour and according to the documentation link on the 31st use of the `>>` operator there is a write that does try to assign 0 to the "out param". – fabian Jun 14 '22 at 15:41
  • @fabian Ahhh... I see what you mean. Yes! Yet another reason not to try to read more numbers than there's room for in the array (which was the point I was trying to make in the first comment). – Ted Lyngmo Jun 14 '22 at 15:44