I have my headers.h
file with a struct so defined:
#ifndef HEADERS
#define HEADERS
#define FILE_ERR -10
#define OK 0
#define TRUE 0
#define FALSE -1
#define SONGS 4
struct song {
char title[31];
char author[31];
int duration_in_sec;
int reps;
};
int load_music_from_file(char*, struct song*);
int write_music_on_file(char*, struct song*);
#endif
I have an input file so written:
we will rock you
queen
2:01
it's my life
bon jovi
3:46
we will rock you
queen
2:01
the show must go on
queen
4:36
I have to read the file and write it out to an output file, modifying the duration in seconds and adding the repetitions of each song in the input file. These are my functions:
#include "headers.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int check_reps(struct song* list, char* title, int n) {
for (int i=0; i<n; i++) {
if (strcmp(list[i].title, title) == 0) {
list[i].reps++;
return TRUE;
}
}
return FALSE;
}
int min_to_sec(int min, int sec) {
return (min * 60) + sec;
}
int load_music_from_file(char* filename, struct song* list) {
FILE* fp = fopen(filename, "r");
if (fp == NULL) return FILE_ERR;
int i = 0;
int min, sec;
char buf_title[31], buf_author[31], buf[6];
while (fgets(buf_title, 31, fp) != NULL && fgets(buf_author, 31, fp) != NULL) {
if (check_reps(list, buf_title, i) == FALSE) {
strncpy(list[i].title, buf_title, 31);
strncpy(list[i].author, buf_author, 31);
if (fgets(buf, 6, fp) != NULL && sscanf(buf, "%d:%d", &min, &sec) == 2) list[i].duration_in_sec = min_to_sec(min, sec);
else return FILE_ERR;
list[i].reps = 1;
printf("%s%s%d\n%d\n\n", list[i].title, list[i].author, list[i].duration_in_sec, list[i].reps);
}
i++;
}
fclose(fp);
return OK;
}
int write_music_on_file(char* filename, struct song* list) {
FILE* fp = fopen(filename, "w");
if (fp == NULL) return FILE_ERR;
for (int i=0; i<SONGS; i++) {
fprintf(fp, "%s%s%d\n%d\n\n", list[i].title, list[i].author, list[i].duration_in_sec, list[i].reps);
}
return OK;
}
This is my main function:
#include "headers.h"
#include <stdio.h>
#include <stdlib.h>
int main() {
struct song* list = (struct song*) malloc(sizeof(struct song) * 4);
char* input = "songs.txt";
printf("loading songs from %s\n", input);
load_music_from_file(input, list);
char* output = "output.txt";
printf("songs written to %s\n\n", output);
write_music_on_file(output, list);
free(list);
return 0;
}
If I cat output.txt
I see:
we will rock you
queen
121
2
it's my life
bon jovi
226
1
0
0
2:01
the show must go on
0
0
I think, the load_music_from_file()
function has a problem, the thing that sets me off is that it works just for a certain number of iterations of the loop.
Side notes:
It seems you have two logical errors in this while loop
while (fgets(buf_title, 31, fp) != NULL && fgets(buf_author, 31, fp) != NULL) {
if (check_reps(list, buf_title, i) == FALSE) {
strncpy(list[i].title, buf_title, 31);
strncpy(list[i].author, buf_author, 31);
if (fgets(buf, 6, fp) != NULL && sscanf(buf, "%d:%d", &min, &sec) == 2) list[i].duration_in_sec = min_to_sec(min, sec);
else return FILE_ERR;
list[i].reps = 1;
printf("%s%s%d\n%d\n\n", list[i].title, list[i].author, list[i].duration_in_sec, list[i].reps);
}
i++;
}
The first one is that if the function check_reps
returns TRUE
then records like that
2:01
are read in the next iteration of the loop as for example a title because these calls of fgets
in the if statement
if (fgets(buf, 6, fp) != NULL && sscanf(buf, "%d:%d", &min, &sec) == 2) list[i].duration_in_sec = min_to_sec(min, sec);
else return FILE_ERR;
do not get the control.
The second logical error is that the variable i
is incremented even when again the function check_reps
returns TRUE
though a new element of the array of structures was not filled in this case.
Also the function load_music_from_file
should report somehow how many elements of the array of structures were actually filled. Instead of using the magic number 4
you should dynamically allocate an array of structures within the function. Though in this case instead of the array it would be better to use a list.
Pay attention to that these macro definitions
#define TRUE 0
#define FALSE -1
are bad and only confuse readers of the code because usually a logical value true is defined as a non-zero value while false as a zero-value.
You could include header <stdbool.h>
and use defined there macros true
, false
and bool
.