I tried decomposing my function, but I'm not sure if I did it right

Question

Originally, this function was embedded into the main function, creating a very cluttered main function. The program replaces tabs with the number of spaces. I'm still confused as to what goes inside the argument lists for my functions and how to pass argc/argv from main into these functions. Did I do this right?

There are some defined variables at the top of the file:

#define OUTFILE_NAME "detabbed"
#define TAB_STOP_SIZE 8
#define NUM_ARGS 2
#define FILE_ARG_IDX 1

Here's my second attempt at it:

void open_file(FILE *inf, FILE *outf, char *in[]) /*I feel like the arguments aren't right  
{                                                   and this function is just opening
                                                    and reading files*/ 
   inf = fopen(in[1], "r");
   outf = fopen(OUTFILE_NAME, "w");

   if (inf == NULL)
   {
      perror(in[1]);
      exit(1);
   }

   else if (outf == NULL)
   {
      perror(OUTFILE_NAME);
      exit(1);
   }

   fclose(inf);
   fclose(outf);
}

void detab(FILE *infile, FILE *outfile, char *argument[]) /* Confused about argument list
{                                                           and this function actually
   char c;                                                  does the detabbing */
   int character_count = 0, i, num_spaces;

   open_file(infile, outfile, argument);                 /* I want to call the previous
                                                          function but again, confused
   while (fscanf(infile, "%c", &c) != EOF)                about the argument list */
   {
      if (c == '\t')
      {
         num_spaces = TAB_STOP_SIZE - (character_count % TAB_STOP_SIZE);

         for (i = 0; i < num_spaces; i++)
         {
            fprintf(outfile, " ");
         }

         character_count += num_spaces;
      }
      else if (c == '\n')
      {

         fprintf(outfile, "\n");
         character_count = 0;
      }
      else
      {
         fprintf(outfile, "%c", c);
         character_count++;
      }
   }

}

int main(int argc, char *argv[])
{
   if (argc < 1)
   {
      fprintf(stderr, "usage: prog file\n");
      exit(1);
   }

   else if (argc < NUM_ARGS)
   {
      fprintf(stderr, "usage: %s file\n", argv[0]);
      exit(1);
   }

   detab(argc, argv);   /* I want to pass argc and argv to the detab function, but I'm
                          having trouble with the argument list */
   return 0;
}

What I need help with, is figuring out what goes in the argument lists of the functions. I think what confuses me is how to get my argument types to match, so that I can pass variables from one function to the other.

Answer 1

Decomposition is not your biggest problem here. Rather careless error checking, the use of old overweighted fscanf() and fprintf() and global variables are. Furthermore, the lack of const correctness in the input filenames, the overly long and verbose variable names and your unawareness of the += and ++ operators are just the bonus. I suppose that's why your code looks like it's bloated (and it is, in fact).

I'd rewrite the function like this:

void detab(const char *in, const char *out, int tabstop)
{
    FILE *inf = fopen(in, "r");
    if (!inf) return;

    FILE *outf = fopen(out, "w");
    if (!outf) {
        fclose(inf);
        return;
    }

    int n = 0;
    int c;
    while ((c = fgetc(inf)) != EOF) {
        if (c == '\t') {
            int pad = tabstop - n % tabstop;

            for (int i = 0; i < pad; i++)
                fputc(' ', outf);

            n += pad;
        } else if (c == '\n') {
            fputc('\n', outf);
            n = 0;
        } else {
            fputc(c, outf);
            n++;
        }
    }

    fclose(inf);
    fclose(outf);
}

If you want to decompose this even further, then you may write a function taking two FILE * and the tab stop as its arguments and it shall contain the while loop only - doing that is left to you as an exercise.

Answer 2

_{Note: This answer was given to an earlier edit of the question. It has changed in the meantime, so this answer may no longer seem relevant.}

Coming from an OOP background, I will focus on one single issue that is there known as Single Responsibility Principle (SRP). I argue that detab (and every other function) should do only one specific thing, but do that thing well.

But it doesn't just "detab", as its name suggests; it also has to extract its actual arguments from the command-line variables argc and argv, which were forced upon it by main:

detab(argc, argv);

main has done some validation before that, but because the command-line is then simply passed to the function, you obviously felt like continuing validation in detab (I also make some additional remarks about violating the SRP below):

void detab(int arg_list, char *array[]) // why ask for arg_list if you don't need it?
{
   …
   infile = fopen(array[1], "r"); // not obvious to caller that only array[1] is relevant 
   …
   if (infile == NULL)
   {
      perror(array[1]);
      exit(1); // should your function really have the power to terminate the program?
   }
   …
}

It would seem much more reasonable to concentrate all the command-line validation and value extraction logic in one place, and the detabbing in another; that is, draw clear boundaries of responsibility. Don't let logic of one function spill over into another!

To me, detab's signature should look more like e.g. this:

void detab(FILE *in, FILE *out);