Member Avatar for jhdobbins

Alright... first off I hate fgets with a passion :)

Anyway, I am wanting to read a file into a 2 dimensional array and then randomly select one of those lines to output...

What am I missing here?

void displayQuestion() {

/* fp is a FILE pointer.  This pointer will read text from 
   the "questions.txt" file */

     int aRandom, i, x;	
     char questions[25][300];	
     FILE *fp = fopen("questions.txt", "rt");

     if (*fp == NULL) {
	printf("Invalid File\n");
     }
	    
     for(x = 0; x < 25; x++){
	
	
     	while( fgets(questions[x], sizeof(questions), fp) != NULL) {
    
     	}
     

     }
	
     aRandom = (rand()%25)+1;

     
    for(i = 0; i < 300; i++)
      {
      temp2=SCISR1;
      temp2=SCIDRL;
     
      while((SCISR1 & 0b10000000) != 0b10000000){}
     
     
      SCIDRL = question[aRandom][i];
      }
          	


     //closes the file
     fclose(fp);


     getButtonPress();

}

Where

for(i = 0; i < 300; i++)
      {
      temp2=SCISR1;
      temp2=SCIDRL;
     
      while((SCISR1 & 0b10000000) != 0b10000000){}
     
     
      SCIDRL = question[aRandom][i];
      }

Prints out the question onto a terminal from a certain processor.

  • 5 Contributors
  • 5 Replies
  • 3K Views
  • 16 Hours Discussion Span
  • Latest Post Latest Post by Salem

Dani AI

Generated

You are very close. Most of the pain here is from control flow and bounds, not from fgets itself. A few concrete fixes, echoing and : check the FILE * itself (not *fp), read each line into a new row once (no nested while inside a for), store how many lines you actually loaded, and pick a random index in [0, count-1]. Also, your array is named questions but you later index question[...]; that mismatch alone will sink the build. Finally, do not transmit a fixed 300 chars; stop at the string terminator so you do not push stale bytes. As hinted, fgets is fine when used correctly.

Example pattern that is safe and simple:

enum { ROWS = 25, COLS = 300 };
static size_t load_questions(const char *path, char a[ROWS][COLS]) {
    FILE *fp = fopen(path, "r");
    if (!fp) return 0;

    size_t n = 0;
    while (n < ROWS && fgets(a[n], COLS, fp)) {
        char *nl = memchr(a[n], '\n', COLS);
        if (nl) *nl = '\0';
        else { int ch; while ((ch = fgetc(fp)) != EOF && ch != '\n') {} }
        ++n;
    }
    fclose(fp);
    return n;
}

Then choose and transmit only what exists:

char questions[ROWS][COLS];
size_t count = load_questions("questions.txt", questions);
if (count == 0) { /* handle missing/empty file */ }

size_t idx = (size_t)rand() % count;   /* 0..count-1 */
for (const char *p = questions[idx]; *p; ++p) {
    /* wait for TX ready, then write *p to SCIDRL */
}

Notes:

  • Use "r"; "rt" is nonstandard but tolerated on some platforms.
  • Seed rand once in main (e.g., srand((unsigned)time(NULL));).
  • If any line may exceed 299 chars, the discard loop above prevents the next fgets from starting mid-line.

Recommended Answers

All 5 Replies

Member Avatar for Member #46692

If you don't like fgets, why don't u read the whole file in as single chars... using the '\n' to distinguish newlines.

a Ha ha.

Edited by Dani because: Fake sig snipped
Member Avatar for dude543

Why do you hate fgets. Its my faviorate function.

char* fgets(char *s, int n, FILE *stream);

fgets reads at most the next n-1 characters into the array s, stopping
if a newline is encounterd; the newline is included in the array, which
is terminated by '\0'. fgets returns s, or NULL if end of file or error
occurs.

replace

fgets(questions[x], sizeof(questions), fp)

with

fgets(questions[x], sizeof(questions[0]), fp)

Notes for your code.
1) If fopen fail on the file, you still calling fgets. Error.
2) replace

aRandom = (rand()%25)+1;

why add 1 ?

array subscript from 0 to 24 !!

Member Avatar for Member #46692

gets() is my favourite function. :lol:

Edited by Dani because: Fake sig snipped
Member Avatar for Narue

>Alright... first off I hate fgets with a passion
How can you hate something you don't completely understand? Don't confuse ignorance of fgets (or C in this case) with any legitimate problems that it has.

Just because you don't know how to use something doesn't mean there's anything wrong with it.

Member Avatar for Salem

> if (*fp == NULL)
You could start here - never mind the rest of it.

Say
if (fp == NULL)

Man, didn't your compiler complain about that line?


> while( fgets(questions[x], sizeof(questions), fp) != NULL)
If you lie about the buffer size, then it's no better than gets()

Besides, if you want to read up to 25 lines, its

for ( x = 0 ;
      x < 25 && fgets(questions[x], sizeof(questions[x]), fp) != NULL )
      x++ ) {
  char *p = strchr( questions[x], '\n' );
  if ( p ) *p = '\0';  /* blow away a newline - if you want to that is */
}
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.