Originally Posted by
thinkabout
The code is working nice for me.
But feel free to give me advice.
1. The specification demands a period and space must follow the line number. Therefore, your format should be:
Code:
fprintf(pFileOut, "%d. %s", nrline, string);
2. Enable warnings and fix the issues that it diagnoses. For example, to make my compiler happy I needed to include <stdio.h> and return a value at the end of put_numbers_on_a_new_file (you forgot the return value for the "success" case).
3a. Remove unused code like putnumbers (I guess that was an experimental version).
3b. Remove excess blank lines. Some blanks help readability; too many harm it.
3c. Remove "t" from the file open mode lines. This character is allowed but unnecessary.
4. There is a subtle error in this part:
Code:
while (fgets(string, sizeof (string), pFile) != NULL) {
fprintf(pFileOut, "%d %s", nrline, string);
nrline++;
}
The problem is that if any line is too long (longer than SIZE-1 characters), the output will be incorrect (try it!). A possible fix is to print out the remainder of a long line afterwards, if needed:
Code:
fprintf(pFileOut, "%d. %s", nrline, string);
/* Print remainder of long lines: */
while (!strchr(string, '\n') &&
fgets(string, sizeof (string), pFile) != NULL)
{
fprintf(pFileOut, "%s", string);
}
5. Use more descriptive argument names for put_numbers_on_a_new_file like inFile and outFile.
6. Supplement fopen error checking with perror to report what went wrong (file not found, permission denied, etc.):
Code:
if ((pFile = fopen(inFile, "r")) == NULL) {
perror(inFile);
return -1;
}
7. You return a status code from put_numbers_on_a_new_file, but you don't use it in main. One possibility:
(main)
Code:
if (put_numbers_on_a_new_file("code.txt", "output.txt") < 0) {
return EXIT_FAILURE;
}
return (EXIT_SUCCESS);
8. Don't hard-code the filenames. That works for an experimental version, but for a testing or production version provide at least a simple command-line interface:
Code:
int main(int argc, char *argv[]) {
if (argc != 4 || strcmp(argv[2],"-o") != 0) {
fprintf(stderr, "Usage: number_lines inFile -o outFile\n");
return EXIT_FAILURE;
}
put_numbers_on_a_new_file(argv[1], argv[3]);
...
Version after my edits:
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define SIZE 100
int put_numbers_on_a_new_file(char * inFile, char * outFile);
int main(int argc, char *argv[]) {
if (argc != 4 || strcmp(argv[2],"-o") != 0) {
fprintf(stderr, "Usage: number_lines inFile -o outFile\n");
return EXIT_FAILURE;
}
if (put_numbers_on_a_new_file(argv[1], argv[3]) < 0) {
return EXIT_FAILURE;
}
return (EXIT_SUCCESS);
}
int put_numbers_on_a_new_file(char * inFile, char * outFile) {
FILE * pFile;
FILE * pFileOut;
char string[SIZE];
int nrline = 1;
if ((pFile = fopen(inFile, "r")) == NULL) {
perror(inFile);
return -1;
}
if ((pFileOut = fopen(outFile, "w")) == NULL) {
perror(outFile);
return -1;
}
while (fgets(string, sizeof (string), pFile) != NULL) {
fprintf(pFileOut, "%d. %s", nrline, string);
while (!strchr(string, '\n') &&
fgets(string, sizeof (string), pFile) != NULL)
{
fprintf(pFileOut, "%s", string);
}
nrline++;
}
return 0;
}