Word wrap with file input & ouput

Question

As this is a very basic word wrap program, it has at least the following 3 limitations :-

1. The file should have a terminating newline.
2. The column number at which the lines are to be wrapped must be greater than or equal to the length of the longest word.
3. Tabs (\t) should not have been used in the lines to be wrapped.

Also, the usage of argv[0] makes this code non-portable.

---

Here is the code :-

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


static char * modified_fgets(char *, const int, FILE *);
static void wrap_line(char *, const int);


int main(int argc, char ** argv)
{

    if (argc != 2)
        printf("Enter this: %s filename\n", argv[0]), exit(EXIT_FAILURE);

    FILE * in;
    if ((in = fopen(argv[1], "r")) == NULL)
        fprintf(stderr, "Can't open %s\n", argv[1]), exit(EXIT_FAILURE);

    FILE * out;
    if ((out = fopen("OUT.txt", "w")) == NULL)
        fprintf(stderr, "Can't create output file\n"), exit(EXIT_FAILURE);

    char s[1024];
    while (modified_fgets(s, 1024, in) != NULL)
        wrap_line(s, 80), fprintf(out, "%s\n", s);

    if ((fclose(in) != 0) || (fclose(out) != 0))
        fprintf(stderr, "Error in closing files\n");

    return EXIT_SUCCESS;

}


// This function reads a line that is terminated by '\n'.
static char * modified_fgets(char * s, const int n, FILE * fp)
{

    char * returnValue;
    returnValue = fgets(s, n, fp);

    int i = 0;

    if (returnValue != NULL)
    {
        while ((s[i] != '\n') && (s[i] != '\0'))
            i++;
        if (s[i] == '\n')
            s[i] = '\0';
        else // s[i] == '\0' (i.e. there are extra characters in the line).
            while (getc(fp) != '\n')
                continue;
    }

    return returnValue;

}


// This function wraps a single line, terminated by '\0' instead of '\n'.
// wrapColumn should be >= the length of the longest word.
// '\t' should not have been used in the line to be wrapped.
static void wrap_line(char * s, const int wrapColumn)
{

    int lastWrap = 0;

    for (int i = 0; i < (int) strlen(s); i++)
        if (i - lastWrap == wrapColumn)
            for (int k = i; k > 0; k--)
                if (s[k] == ' ')
                {
                    s[k] = '\n';
                    lastWrap = k+1;
                    break;
                }

}

---

Here is a sample input file :-

Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.
Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of "de Finibus Bonorum et Malorum" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, "Lorem ipsum dolor sit amet..", comes from a line in section 1.10.32.
It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout. The point of using Lorem Ipsum is that it has a more-or-less normal distribution of letters, as opposed to using 'Content here, content here', making it look like readable English. Many desktop publishing packages and web page editors now use Lorem Ipsum as their default model text, and a search for 'lorem ipsum' will uncover many web sites still in their infancy. Various versions have evolved over the years, sometimes by accident, sometimes on purpose (injected humour and the like).

---

Here is the desired output file :-

Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem
Ipsum has been the industry's standard dummy text ever since the 1500s, when an
unknown printer took a galley of type and scrambled it to make a type specimen
book. It has survived not only five centuries, but also the leap into electronic
typesetting, remaining essentially unchanged. It was popularised in the 1960s
with the release of Letraset sheets containing Lorem Ipsum passages, and more
recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots
in a piece of classical Latin literature from 45 BC, making it over 2000 years
old. Richard McClintock, a Latin professor at Hampden-Sydney College in
Virginia, looked up one of the more obscure Latin words, consectetur, from a
Lorem Ipsum passage, and going through the cites of the word in classical
literature, discovered the undoubtable source. Lorem Ipsum comes from sections
1.10.32 and 1.10.33 of "de Finibus Bonorum et Malorum" (The Extremes of Good and
Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of
ethics, very popular during the Renaissance. The first line of Lorem Ipsum,
"Lorem ipsum dolor sit amet..", comes from a line in section 1.10.32.
It is a long established fact that a reader will be distracted by the readable
content of a page when looking at its layout. The point of using Lorem Ipsum is
that it has a more-or-less normal distribution of letters, as opposed to using
'Content here, content here', making it look like readable English. Many desktop
publishing packages and web page editors now use Lorem Ipsum as their default
model text, and a search for 'lorem ipsum' will uncover many web sites still in
their infancy. Various versions have evolved over the years, sometimes by
accident, sometimes on purpose (injected humour and the like).

---

Edit :-

I guess the first limitation is easy to fix. Using while (((c = getc(fp)) != '\n') && (c != EOF)) instead of while (getc(fp) != '\n') in the modified_fgets() function does the trick, where c is declared as an integer beforehand.

Answer 1

Here are some things that may help you improve your code.

Use a named const for fixed values

The buffer size is fixed a 1024 bytes, and the number 1024 appears in two places. I'd recommend making that a named constant instead, using the C11 const keyword C99 variable length arrays (thanks to chux for the correction!) or via a #define.

/* using variable length array */
const unsigned buffsize = 1024;
/* alternative using #define */
#define BUFFSIZE 1024

Use curly braces for clarity

The only place the curly braces ({}) are needed for control flow in this program is the innermost if within wrap_line(), but using them for each control structure may help with clarity and readability for human programmers. For that reason, I'd recommend their use.

Don't abuse the comma operator

This line doesn't really need a comma operator:

if (argc != 2)
    printf("Enter this: %s filename\n", argv[0]), exit(EXIT_FAILURE);

If you follow the previous advice, and write for clarity, that would look like this instead:

if (argc != 2) {
    printf("Enter this: %s filename\n", argv[0]); 
    exit(EXIT_FAILURE);
}

It's too easy to overlook the exit() at the end.

Apply error checking consistently

If the final fclose() for either file fails, the program still exits with EXIT_SUCCESS which does not seem consistent with the other error handling in main(). Additionally, because of logical operator short-circuit evaluation, if the first fclose(in) results in a non-zero return, the second fclose() will not be called. In this particular case, the program would exit and the operating system would flush and close the file, but again, that's handling the errors inconsistently.

Think of the user

There are a few things about this implementation that could be improved for the user's experience. The first is to either allow specifying the output file name. One way this might be done is to simply read from stdin and write to stdout as many programs do in Linux. This allows one to pipe from the output of one program to the input of another, for example. The second thing is to adjust how the word wrapping is done. If the user feeds this program a line that is longer than 1024 characters, this program will silently discard the excess length to the next newline character. At the very least, signalling that this has happened might be better. I'd prefer instead if the program didn't have such an arbitrary limit.

Fix the bug

Try changing your output line length to 5 and check the output. I'd characterize it as a bug.

Consider an alternative approach

An alternative approach would be to read in characters until either a newline or 80 characters were read. If a newline is read, simply print the buffer to that point. If 80 characters were read, back up from there and insert a break at the first whitespace, keeping the remainder of the buffer to append to for the next line. If no whitespace was found, consider whether to insert a break anyway or printing the line as is and implement your choice.

Answer 2

Readability

Your (ab)use of the comma operator is detrimental to program readability. It is extremely easy for a reader of your code to miss the fact that you have two expressions on one line. Reformat your code to have one statement (expression) on a line, and make full use of curly braces to make the code easy for someone to casually follow.

For example:

    if (argc != 2) {
        printf("Enter this: %s filename\n", argv[0]);
        exit(EXIT_FAILURE);
    }

The lack of curly braces in wrap_line makes the code a bit harder to follow because of all that indentation. A reader has to consider if that is one statement on multiple lines or multiple statements. The additional curly braces would make it obvious to a reader that these are distinct statements.

A minor possible issue is the excessive use of blank lines. Having a blank line after the opening { of a function (before any code) can have the effect of disconnecting that code form the function declaration. (The same applies to the closing } of the function.)

Performance

The modified_fgets could handle long lines better (reading in a block of characters rather than looking at them one at a time). Adding this would then show a possible way to handle (and wrap) those long lines, as well as files that don't end in a newline.

wrap_line is a big bottleneck. This can all be handled in a single pass thru the string, but your method is O(N²) because you call strlen every time. You can walk thru the string, one character at a time. Keep track of where you start, the number of characters you've looked at, and when the last space was. Then, when you gone as far as you can before you have to wrap, you can update that last space, move the start indicator, update the length, and keep processing. (This would also allow for adding handling of "words" longer than the wrap length.)