Program structure

[John L]

Hello!

I'm somewhat new to C, and the problem is I'm not sure how to structure my
programs. I keep ending up with massively long main() functions that look
like this:

int main(void)
{
declarations;

if (file_didnt_open_or_something) {
return EXIT_FAILURE;
}

if (mem_didnt_allocate_or_something) {
close file;
return EXIT_FAILURE;
}

if (another_malloc_failed_or_something) {
free mem1;
close file;
return EXIT_FAILURE;
}

if (fread_failed_or_something) {
free mem2;
free mem1;
close file;
return EXIT_FAILURE;
}

/* etc. */

free(mem2)
free(mem1);
fclose(file);
return 0;
}

I'm pretty sure I'm not using C's control flow constructions intelligently
at all, since this reminds me of x86 assembly language! How would you guys
arrange a program like the one here?

Advice much appreciated
John

 

[Mike Wahler]

Hello!

I'm somewhat new to C, and the problem is I'm not sure how to structure my
programs. I keep ending up with massively long main() functions

Refactor. Create small functions that perform a single
task, and call them from main (and/or each other).

that look
like this:

int main(void)
{
declarations;

if (file_didnt_open_or_something) {
return EXIT_FAILURE;
}

if(try_to_open_file(filename) == NULL) /* (you write the
'try_to...' function */
return EXIT_FAILURE;

etc.

I'm pretty sure I'm not using C's control flow constructions intelligently

The only control flow you're using (other than calling
library functions) is the 'if' statement and 'return'
statement. They look fine to me. Your 'main()' isnt'
really all that long, and doing it all in main() is
often done for short simple programs.

at all, since this reminds me of x86 assembly language!

Not me.

How would you guys
arrange a program like the one here?

Advice much appreciated

See above.

-Mike

 

[David Rubin]

Hello!

I'm somewhat new to C, and the problem is I'm not sure how to structure my
programs. I keep ending up with massively long main() functions that look
like this:

[snip - code]

This is actually pretty reasonable. For example, a program that filters
a file (i.e., processes it such as tab, uncomment, xref (count
word/letter references/frequencies)) is often structured like this:

int
main(int argc, char **argv)
{
/* declare variables */

/* process arguments (changes argc/argv) */

if(argc){
for(i=0; i < argc; i++){
if((fp=fopen(argv, "r")) == 0){
perror(argv);
exit(1);
}
filter(fp); /* "filter" is often the name of the program
*/
fclose(fp);
}
}else
filter(stdin);
return 0;
}

The idea is that file operations (open/close) are done in the same
scope, and the meat of the program is done by a function which takes an
"open file" as an argument. Processing command line options is
relatively straightforward, so this can be done in main(), assisted by
global option variables. For example, if the usage is

filter [-sdq] [-h n] file...

then you might declare global variables

int sopt = 0;
int dopt = 0;
int qopt = 1;
int hopt = 0;
int harg = 0;

This lets you easily process arguments in main() but use them in the
appropriate lower-level function.

/david

 

[Nick Austin]

Hello!

I'm somewhat new to C, and the problem is I'm not sure how to structure my
programs. I keep ending up with massively long main() functions that look
like this:

int main(void)
{
declarations;

if (file_didnt_open_or_something) {
return EXIT_FAILURE;
}

if (mem_didnt_allocate_or_something) {
close file;
return EXIT_FAILURE;
}

if (another_malloc_failed_or_something) {
free mem1;
close file;
return EXIT_FAILURE;
}

if (fread_failed_or_something) {
free mem2;
free mem1;
close file;
return EXIT_FAILURE;
}

/* etc. */

free(mem2)
free(mem1);
fclose(file);
return 0;
}

I'm pretty sure I'm not using C's control flow constructions intelligently
at all, since this reminds me of x86 assembly language! How would you guys
arrange a program like the one here?

Use a variable for the return code. Also it is safe to call free
with a NULL pointer, so just make sure that all pointer variables
are initialised to NULL.


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

int main(void)
{
int result = EXIT_FAILURE;
char *mem1 = NULL;
char *mem2 = NULL;
FILE *file = NULL;

file = fopen( "test", "r" );
if ( file ) {
mem1 = malloc( 100 );
if ( mem1 ) {
mem2 = malloc( 100 );
if ( mem2 ) {
if ( !fread_failed_or_something ) {
result = EXIT_SUCCESS;
/* etc. */
}
}
}
}

free( mem2 );
free( mem1 );

if ( file )
fclose( file );

return result;
}

In this particular case I consider it safe to open the file and
allocate memory before any checks for failure. So it would be
simpler to do:

file = fopen( "test", "r" );
mem1 = malloc( 100 );
mem2 = malloc( 100 );

if ( file && mem1 && mem2 ) {
if ( !fread_failed_or_something ) {
result = EXIT_SUCCESS;
/* etc. */
}
}

Nick.

 

[Richard Heathfield]

Hello!

I'm somewhat new to C, and the problem is I'm not sure how to structure my
programs. I keep ending up with massively long main() functions that look
like this:

int main(void)
{
declarations;

if (file_didnt_open_or_something) {
return EXIT_FAILURE;
}

if (mem_didnt_allocate_or_something) {
close file;
return EXIT_FAILURE;
}

if (another_malloc_failed_or_something) {
free mem1;
close file;
return EXIT_FAILURE;
}

if (fread_failed_or_something) {
free mem2;
free mem1;
close file;
return EXIT_FAILURE;
}

/* etc. */

free(mem2)
free(mem1);
fclose(file);
return 0;
}

I'm pretty sure I'm not using C's control flow constructions intelligently
at all, since this reminds me of x86 assembly language! How would you guys
arrange a program like the one here?

A little bit like this:

int main(void)
{
int rc = 0;
other declarations;

if((rc = FileOpenedOK()) == 0)
{
if ((rc = AllocationSucceeded()) == 0)
{
if((rc = NextAllocationSucceeded()) == 0)
{
rc = DoTheThingYouWanted();
free(SecondLotOfMemory);
}
free(FirstLotOfMemory);
}
fclose(filepointer);
}
return rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}

The trick is to decide at which point it makes sense to split off the guts
of the function into a separate function. I'm not uncomfortable with the
level of nesting shown above, but I wouldn't want it to get too much
deeper.