What do you think about the code?

J

jaso

Can you give any comments on this code?
I used one goto, is it bad?

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

#define NOT_NULL 1
#define NUMERIC 2
#define MULTI_LINE 4

/* Describes one field of a record */
struct field_desc {
char *name;
int length;
int type;
};

#define FIELD_PROMPT "%s (%d): "

struct record {
size_t count;
struct field_desc *desc;
char **entries;
};

struct field_desc movies_desc[] = {
{ "id", 13, NOT_NULL | NUMERIC },
{ "title", 50, NOT_NULL },
{ "genre", 15, NOT_NULL },
{ "length", 3, NOT_NULL | NUMERIC },
{ "description", 65535, MULTI_LINE },
{ "owner", 10, NOT_NULL },
{ "available", 1, NOT_NULL | NUMERIC },
{ "customer", 30, 0 } };

#define MOVIES_LENGTH (sizeof(movies_desc)/sizeof(movies_desc[0]))

char *readl(size_t length, int multiline, char stop)
{
const size_t bufsize = 32;
int c, lastc;
size_t index;
size_t size;
char *str, *tmp;

size = 0;
str = NULL;

if (multiline)
lastc = '\n';
else
lastc = 0;

index = 0;
while ((c = getchar()) != EOF && index < length) {
if (index + 1 >= size) {
size += bufsize;
if (size > length + 1)
size = length + 1;

tmp = realloc(str, size);
if (tmp == NULL) {
if (str)
free(str);
return NULL;
}
str = tmp;
}

/* Will only be true if multiline */
if (c == stop && lastc == '\n') {
if (stop == '\n')
break;
if ((c = getchar()) == '\n' || c == EOF)
break;
else {
ungetc(c, stdin);
c = stop;
}
}
if (c == '\n' && !multiline)
break;

assert(index + 1 < size);
str[index++] = c;
lastc = c;
}
if (c == EOF && index == 0)
return NULL;

assert(str != NULL);
assert(index < size);
str[index] = '\0';

/* Remove trailing '\n's (multiline only) */
if (index != 0 && lastc == '\n')
str[index - 1] = '\0';

/* Remove remaining characters on the line. */
while(c != '\n' && c != EOF)
c = getchar();

return str;
}

/* This function assumes sticky eof. That means,
* if an EOF is encountered, it will be read again
* from getchar() at the next call to this function. */
char *read_multiline(size_t length)
{
return readl(length, 1, '\n');
}

/* Reads one line of max length and discards additional
* characters on the line. Intended for interactive use */
char *read_line(size_t length)
{
return readl(length, 0, 0);
}

int isnumeric(const char *str)
{
if (!str)
return 0;
while (*str) {
if (!isdigit(*str) && *str != '\n')
return 0;
str++;
}
return 1;
}

static int validate_entry(const char *entry, struct field_desc desc)
{
if ((desc.type & NUMERIC) && !isnumeric(entry)) {
fprintf(stderr, "Error: Value must be numeric\n");
return -1;
}
if ((desc.type & NOT_NULL) && entry[0] == '\0') {
fprintf(stderr, "Error: Value must not be null\n");
return -1;
}

return 0;
}

static void field_prompt(struct field_desc desc)
{
printf(FIELD_PROMPT, desc.name, desc.length);
fflush(stdout);
}

static int get_field(char **entry, struct field_desc desc)
{
if (desc.type & MULTI_LINE)
*entry = read_multiline(desc.length);
else
*entry = read_line(desc.length);

if (*entry == NULL)
return EOF;

return 0;
}

int set_record_type(struct record *r, struct field_desc *desc, size_t
length)
{
size_t i;
r->count = length;
r->desc = desc;
r->entries = malloc(length * sizeof(char *));
if (r->entries == NULL)
return -1;
/* Init all data to NULL */
for (i = 0; i < length; i++)
r->entries = NULL;
return 0;
}

/* Fills record with input from user */
int fill_record(struct record r)
{
size_t i;
for (i = 0; i < r.count; i++) {

field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;

while (validate_entry(r.entries, r.desc) != 0) {
free(r.entries);
field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;
}
}

return 0;

atEOF:
while (i--) {
free(r.entries);
r.entries = NULL;
}
return EOF;
}

void print_record(struct record r)
{
size_t i;
for (i = 0; i < r.count; i++) {
printf("%s:\"%s\"\n", r.desc.name, r.entries);
}
}

void free_record(struct record r)
{
size_t i;

for (i = 0; i < r.count; i++) {
if (r.entries)
free(r.entries);
}
free(r.entries);
}

int main(void)
{
struct record movies_record;

set_record_type(&movies_record, movies_desc, MOVIES_LENGTH);

fill_record(movies_record);

print_record(movies_record);

free_record(movies_record);

return EXIT_SUCCESS;
}
 
M

milhous.30

you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Can you give any comments on this code?
I used one goto, is it bad?

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

#define NOT_NULL 1
#define NUMERIC 2
#define MULTI_LINE 4

/* Describes one field of a record */
struct field_desc {
char *name;
int length;
int type;
};

#define FIELD_PROMPT "%s (%d): "

struct record {
size_t count;
struct field_desc *desc;
char **entries;
};

struct field_desc movies_desc[] = {
{ "id", 13, NOT_NULL | NUMERIC },
{ "title", 50, NOT_NULL },
{ "genre", 15, NOT_NULL },
{ "length", 3, NOT_NULL | NUMERIC },
{ "description", 65535, MULTI_LINE },
{ "owner", 10, NOT_NULL },
{ "available", 1, NOT_NULL | NUMERIC },
{ "customer", 30, 0 } };

#define MOVIES_LENGTH (sizeof(movies_desc)/sizeof(movies_desc[0]))

char *readl(size_t length, int multiline, char stop)
{
const size_t bufsize = 32;
int c, lastc;
size_t index;
size_t size;
char *str, *tmp;

size = 0;
str = NULL;

if (multiline)
lastc = '\n';
else
lastc = 0;

index = 0;
while ((c = getchar()) != EOF && index < length) {
if (index + 1 >= size) {
size += bufsize;
if (size > length + 1)
size = length + 1;

tmp = realloc(str, size);
if (tmp == NULL) {
if (str)
free(str);
return NULL;
}
str = tmp;
}

/* Will only be true if multiline */
if (c == stop && lastc == '\n') {
if (stop == '\n')
break;
if ((c = getchar()) == '\n' || c == EOF)
break;
else {
ungetc(c, stdin);
c = stop;
}
}
if (c == '\n' && !multiline)
break;

assert(index + 1 < size);
str[index++] = c;
lastc = c;
}
if (c == EOF && index == 0)
return NULL;

assert(str != NULL);
assert(index < size);
str[index] = '\0';

/* Remove trailing '\n's (multiline only) */
if (index != 0 && lastc == '\n')
str[index - 1] = '\0';

/* Remove remaining characters on the line. */
while(c != '\n' && c != EOF)
c = getchar();

return str;
}

/* This function assumes sticky eof. That means,
* if an EOF is encountered, it will be read again
* from getchar() at the next call to this function. */
char *read_multiline(size_t length)
{
return readl(length, 1, '\n');
}

/* Reads one line of max length and discards additional
* characters on the line. Intended for interactive use */
char *read_line(size_t length)
{
return readl(length, 0, 0);
}

int isnumeric(const char *str)
{
if (!str)
return 0;
while (*str) {
if (!isdigit(*str) && *str != '\n')
return 0;
str++;
}
return 1;
}

static int validate_entry(const char *entry, struct field_desc desc)
{
if ((desc.type & NUMERIC) && !isnumeric(entry)) {
fprintf(stderr, "Error: Value must be numeric\n");
return -1;
}
if ((desc.type & NOT_NULL) && entry[0] == '\0') {
fprintf(stderr, "Error: Value must not be null\n");
return -1;
}

return 0;
}

static void field_prompt(struct field_desc desc)
{
printf(FIELD_PROMPT, desc.name, desc.length);
fflush(stdout);
}

static int get_field(char **entry, struct field_desc desc)
{
if (desc.type & MULTI_LINE)
*entry = read_multiline(desc.length);
else
*entry = read_line(desc.length);

if (*entry == NULL)
return EOF;

return 0;
}

int set_record_type(struct record *r, struct field_desc *desc, size_t
length)
{
size_t i;
r->count = length;
r->desc = desc;
r->entries = malloc(length * sizeof(char *));
if (r->entries == NULL)
return -1;
/* Init all data to NULL */
for (i = 0; i < length; i++)
r->entries = NULL;
return 0;
}

/* Fills record with input from user */
int fill_record(struct record r)
{
size_t i;
for (i = 0; i < r.count; i++) {

field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;

while (validate_entry(r.entries, r.desc) != 0) {
free(r.entries);
field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;
}
}

return 0;

atEOF:
while (i--) {
free(r.entries);
r.entries = NULL;
}
return EOF;
}

void print_record(struct record r)
{
size_t i;
for (i = 0; i < r.count; i++) {
printf("%s:\"%s\"\n", r.desc.name, r.entries);
}
}

void free_record(struct record r)
{
size_t i;

for (i = 0; i < r.count; i++) {
if (r.entries)
free(r.entries);
}
free(r.entries);
}

int main(void)
{
struct record movies_record;

set_record_type(&movies_record, movies_desc, MOVIES_LENGTH);

fill_record(movies_record);

print_record(movies_record);

free_record(movies_record);

return EXIT_SUCCESS;
}
 
A

Andrew Poelstra

you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Don't top-post. I've eliminated what you quoted because it isn't really
relevant to your point.

What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");


/* End */

Or, without goto:

/* Start */

int status = 0;

while (test[0])
{
while (test[1])
{
while (test[2])
{
if (test[3])
{
status = 1;
break;
}
}
if (status)
break;
}
if (status)
break;
}

puts ("Escaped from loop.");

/* End */
 
I

Ian Collins

Andrew said:
What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");


/* End */

Or, without goto:

/* Start */

int status = 0;

while (test[0])
{
while (test[1])
{
while (test[2])
{
if (test[3])
{
status = 1;
break;
}
}
if (status)
break;
}
if (status)
break;
}

puts ("Escaped from loop.");

/* End */
You forgot

int ok = 1;

while (ok && test[0])
while (ok && test[1])
while (ok && test[2])
{
if (test[3])
ok = 0;
}
puts ("Escaped from loop.");
 
T

Tom St Denis

you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Common newbieism. Try code like

----
if (func1() != OK) { goto ERR; }
stmt1;
if (func2() != OK) { goto ERR; }
stmt2;
if (func3() != OK) { goto ERR; }
stmt3;

return OK;
ERR:
/* err occured, deal with it */
---

versus the "smart" way

---
if (func1() != OK) { deal_with_error; }
stmt1;
if (func2() != OK) { deal_with_error; }
stmt2;
if (func3() != OK) { deal_with_error; }
stmt3;
----

Where deal_with_error could be say 10-25 lines of cleanup code
depending on the function. Now pretend you're writing a function that
has 40 blocks like that [say doing a lot of failable bignum math
calls].

goto is no more evil than the "do" keyword. I could just as easily
write incomprehensible switch statements too...

switch (mychar == 'Y') {
case 0: do_something();
default: or_not();
}

etc, etc, etc.

Anyone who claims "goto" is evil basically hasn't written a
sufficiently complicated enough program yet.

Tom
 
B

Bill Pursell

(e-mail address removed) top-posted:
you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Can you give any comments on this code?
I used one goto, is it bad?
/* Fills record with input from user */
int fill_record(struct record r)
{
size_t i;
for (i = 0; i < r.count; i++) {

field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;

while (validate_entry(r.entries, r.desc) != 0) {
free(r.entries);
field_prompt(r.desc);
if (get_field(&r.entries, r.desc) == EOF)
goto atEOF;
}
}

return 0;

atEOF:
while (i--) {
free(r.entries);
r.entries = NULL;
}
return EOF;
}


This is a perfectly acceptable use of goto, and IMO is the
correct way to code this. The label is at the end of the block
and is used to bail out of the inner loop and do clean-up.
There's nothing wrong with that, and there's no point in
bending over backwards to write convoluted error handling
just because someone once wrote that goto is bad.
 
R

Richard Bos

you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Absolutism is bad for your soul, especially when it's top-posted and
wrongly applied, like yours.

Richard
 
G

goose

you can write any code that uses a goto, without a goto.

duffs device.
Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Yup, goto and longjmp/setjmp make code so hard to read
that they're considered harmful!

/sarcasm

Readability and unconditional jumps are not necessarily
mutually exclusive, you know.

goose

<snipped>
 
J

James Dow Allen

jaso said:
Can you give any comments on this code?
I used one goto, is it bad?

Sometimes there's a way to rearrange the logic
to eliminate the goto. This will usually make
the code slightly more maintainable but, in your
example, would hardly be worth the bother.

Let me follow up on my own 'goto' query
from almost a year ago. The present comp.lang.c
players seem intelligent and open-minded.
I hope they'll humor me and "vote" on my case.

Tim Rentsch submitted only a "pseudo-code" synopsis
(Usenet msg <[email protected]>),
but that's just as well: I've abbreviated my
own code the same way; lengthy but irrelevant
details are removed and an easy comparison can
be made.

I've never denied that goto's are *bad*.
The point is that other things (longjmp(),
redundant tests, break, inside-loop state changes,
etc.) are also bad to varying degrees and one
seeks compromise. In the particular code below,
the 'goto' may seem *horrendously* unstructured
but study reveals a strict and elegant structure,
albeit a highly non-standard one.

* * * * * * * * * * * * *

I'll show the actual pseudocode routines in
a moment, but I found a simple way to abbreviate
them further and highlight the different flow
construction. The details of Tim's routine are
identical to mine except for the associated flow
and looping controls. By deleting all *identical*
lines, we see exactly what changes Tim required
to eliminate the 'goto':

Version 1 (omitting parts identical to Version 2):
ITERATE through tricks until outcome is decided
ITERATE clockwise through players
try_again:
ITERATE through tricks backwards until done
ITERATE counterclockwise through players
If Need to Backtrack
GOTO try_again;

Version 2 (omitting parts identical to Version 2):
ITERATE FOREVER
ITERATE FOREVER
If trick complete
ADVANCE STATE to next trick;
If outcome decided
BREAK;
Else ADVANCE to next player;
ITERATE FOREVER
If trick empty
If done
return or exit;
REGRESS STATE to previous trick;
Else REGRESS to previous player;
If Need to Backtrack
BREAK;


My "Iterates" are simple for-loops to iterate through
the 13 tricks of a whist hand, or the four cards played
to a trick. Tim couldn't use these (he'd need 'goto'
or C's non-existent 'break 2') so resorted to inside-loop
state changes. For me, replacing the clear and elegant
'for (;;iteration)' construct with a forever-loop and
inside-loop state changes is highly undesirable.
Anyway, forever-loops are always at least *slightly*
bad: "nothing is forever" but the ending
condition is hidden inside the loop with an
If ... Break or If ... Return.

Please compare the two fragments and decide which is
"better." On balance, one might say that the first
fragment is similar to the second but replaces 2 breaks,
4 state changes and an inside-loop return(*) with just
one for-loop and one goto. (* -- I don't think a return
or exit() inside a loop is that big of a problem, but
it is objectionable and the fact that the first fragment
naturally finishes at its end does seem nice.)

* * * * * * * * * * * * *

Version 1:
Initialize;
ITERATE through tricks until outcome is decided {
Set trick to empty;
ITERATE clockwise through players {
Determine available legal plays;
try_again:
Choose a legal card, and play it to trick;
}
Determine trick's winner, adjusting score;
}
ITERATE through tricks backwards until done {
ITERATE counterclockwise through players {
Retract played card;
Delete it from available plays;
If (card led to defeat)
&& (alternatives remain) {
GOTO try_again;
}
}
}
Print solution and exit;


Version 2:
Initialize;
ITERATE FOREVER {
ITERATE FOREVER {
Determine available legal plays;
Choose a legal card, and play it to trick;
If trick complete {
Determine trick's winner, adjusting score;
If outcome decided {
BREAK;
}
ADVANCE STATE to next trick;
Set trick to empty;
} Else ADVANCE to next player;
}
ITERATE FOREVER {
If trick empty {
If done {
Print solution and exit;
}
REGRESS STATE to previous trick;
} Else REGRESS to previous player;
Retract played card;
Delete it from available plays;
If (card led to defeat)
&& (alternatives remain) {
BREAK;
}
}
}


Which code is more readable?

* * * * * * * * * * * * *

BTW, peoply rightly complain that the problem
with 'goto' is the need to grok all references
to the associated label but, in any non-trivial
case, isn't an editor search for all occurrences
of that label the first thing you do? No one's
trying to defend spaghetti code with a maze of
goto's.

During the Jurassic Era many of us didn't have
editors but assemblers printed an index of
references. I was always baffled when someone
overlooked an identifier's purpose: when making
a change I always checked the reference index
when I had any uncertainty about an identifier.

* * * * * * * * * * * * *

History of this "Code Fragment":

The "code fragment" is actually a complete program to
solve a non-trivial problem (double-dummy whist analysis),
but I call it "fragment" because I've used "pseudo-code"
to reduce it to a few lines.

Version 1 (compilable C, not this pseudocode) and some
further comments are on display at
http://tinyurl.com/2452h/dbldum.htm

James Dow Allen

- - - - - - - - - - -
Tell us, Senator, what is your opinion of alcohol?

``Well, if by alcohol you mean that hearty spirit which brings laughter
and
livelihood to social gatherings, which lubricates the gears of
business
transactions, which makes life's hardships easier to bear, then I say
hurrah for it, and I support its legal sale and consumption.

``However, if by alcohol you mean that evil essence which robs the
family
budget, which is a toxin to the body and the mind, which causes men
to
assault one another and to fail in their professions, then I say fie
on
it, and I will work with all my power to prevent its sale and
consumption.''

- - - - - - - - - - -
 
J

Joe Wright

Tom said:
you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Common newbieism. Try code like

----
if (func1() != OK) { goto ERR; }
stmt1;
if (func2() != OK) { goto ERR; }
stmt2;
if (func3() != OK) { goto ERR; }
stmt3;

return OK;
ERR:
/* err occured, deal with it */
---

versus the "smart" way

---
if (func1() != OK) { deal_with_error; }
stmt1;
if (func2() != OK) { deal_with_error; }
stmt2;
if (func3() != OK) { deal_with_error; }
stmt3;
----

Where deal_with_error could be say 10-25 lines of cleanup code
depending on the function. Now pretend you're writing a function that
has 40 blocks like that [say doing a lot of failable bignum math
calls].

goto is no more evil than the "do" keyword. I could just as easily
write incomprehensible switch statements too...

switch (mychar == 'Y') {
case 0: do_something();
default: or_not();
}

etc, etc, etc.

Anyone who claims "goto" is evil basically hasn't written a
sufficiently complicated enough program yet.

Tom
I don't claim goto evil except that it destroys program structure. I
have only ever used goto in little test programs to see how it works. I
am much more a fan of break and continue to defeat looping blocks.
 
M

milhous.30

Programs written with the goto, loose structure, and according to
Dijkstra you can write anything without the goto. The problem is not
with simple cases, but with the more complicated cases.

BTW, requiring top posting is more absolutist than elmimiating goto's.
 
M

milhous.30

Andrew said:
you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Don't top-post. I've eliminated what you quoted because it isn't really
relevant to your point.

What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");


/* End */

Or, without goto:

/* Start */

int status = 0;

while (test[0])
{
while (test[1])
{
while (test[2])
{
if (test[3])
{
status = 1;
break;
}
}
if (status)
break;
}
if (status)
break;
}

puts ("Escaped from loop.");

/* End */

while(test[0] || test[1] || test[2])
{
if(test[3])
break;
}
 
R

Richard Heathfield

Andrew Poelstra said:

What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");

Now add sixty to a hundred lines of actual code, ten lines of comments, and
a couple of years of maintenance, and /then/ ask me whether this is easy to
read.
Or, without goto:

Unstructured version snipped. The break keyword is just goto in a false
moustache.

In practice, status objects are cheap and readable. If they are becoming
unreadable, it's a sign that your function is too big. One huge advantage
of a modular design with strict avoidance of goto, non-documentary
continue, and switchless break, is that it is really easy to refactor a
function that has outgrown its readability. Just chop out the innermost
loop, dump it into a new function, and that's typically almost all that you
have to do. When the control flow is hopping all over the place like a frog
on his wedding night, code simplification is far more difficult to achieve.
 
A

Andrew Poelstra

Andrew said:
you can write any code that uses a goto, without a goto.

Goto is disliked, as it leads to code that is harder to read. You
should try to eliminate goto's as it will make somebody(other then you)
able to edit your code.

Don't top-post. I've eliminated what you quoted because it isn't really
relevant to your point.

What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");


/* End */

Or, without goto:

/* Start */

int status = 0;

while (test[0])
{
while (test[1])
{
while (test[2])
{
if (test[3])
{
status = 1;
break;
}
}
if (status)
break;
}
if (status)
break;
}

puts ("Escaped from loop.");

/* End */

while(test[0] || test[1] || test[2])
{
if(test[3])
break;
}

Did I need to clarify that there was other code in there?
Convert this to one while loop:
while (test[0]) {
puts ("Test[0]");
while (test[1]) {
puts ("Test[1]");
while (test[2]) {
puts ("Test[2]");
if (test[3])
goto goatee;
}
}
}

goatee:
puts ("Test[3] or !Test[0]");

(That's also a reason for why I don't use that bracing style.)

There were other, much better ways, posted for me getting around
using a goto in that situation.
 
A

Andrew Poelstra

Andrew Poelstra said:

What is easier to read:
/* Start */

while (test[0])
while (test[1])
while (test[2])
{
if (test[3])
goto outOfLoop;
}

outOfLoop:
puts ("Escaped from loop.");

Now add sixty to a hundred lines of actual code, ten lines of comments, and
a couple of years of maintenance, and /then/ ask me whether this is easy to
read.
Or, without goto:

Unstructured version snipped. The break keyword is just goto in a false
moustache.

In practice, status objects are cheap and readable. If they are becoming
unreadable, it's a sign that your function is too big. One huge advantage
of a modular design with strict avoidance of goto, non-documentary
continue, and switchless break, is that it is really easy to refactor a
function that has outgrown its readability. Just chop out the innermost
loop, dump it into a new function, and that's typically almost all that you
have to do. When the control flow is hopping all over the place like a frog
on his wedding night, code simplification is far more difficult to achieve.

Since as a maintenance programmer you've probably had to deal with a lot of
similar code, suppose you were writing a parser and you did this:

c = get_input();

if (we_are_in_a_quote && c != '\"')
goto skip_parsing;

switch (c)
{
/* Parsing code here */
}

skip_parsing:
/* Return parsed text to callee. */


Would that be an appropriate use of goto? Otherwise I've found that I end up
with if statements around most every single case block. (I can't do it around
the whole switch because I don't care if I'm in quotes when checking for the
quote character and the escape character).
 
R

Richard Heathfield

Andrew Poelstra said:

Since as a maintenance programmer you've probably had to deal with a lot
of similar code, suppose you were writing a parser and you did this:

c = get_input();

if (we_are_in_a_quote && c != '\"')
goto skip_parsing;

switch (c)
{
/* Parsing code here */
}

skip_parsing:
/* Return parsed text to callee. */


Would that be an appropriate use of goto?

I wouldn't use goto in that situation, no. I'd use a state machine
architecture, looping while state != DONE, and inside the loop I'd use a
switch statement, one case per state.
 
I

Ian Collins

Andrew said:
Since as a maintenance programmer you've probably had to deal with a lot of
similar code, suppose you were writing a parser and you did this:

c = get_input();

if (we_are_in_a_quote && c != '\"')
goto skip_parsing;

switch (c)
{
/* Parsing code here */
}

skip_parsing:
/* Return parsed text to callee. */


Would that be an appropriate use of goto? Otherwise I've found that I end up
with if statements around most every single case block. (I can't do it around
the whole switch because I don't care if I'm in quotes when checking for the
quote character and the escape character).
I agree 100% with Richard's comments, the cleaner the code structure,
the easier it is to refactor. That's why my last team banned goto outright.

In the parser example, I would call a function for each case and include
the test for quotes explicitly where required. I think this makes the
code clearer.
 
F

Frederick Gotham

Ian Collins posted:

I agree 100% with Richard's comments, the cleaner the code structure,
the easier it is to refactor. That's why my last team banned goto
outright.


I find "goto" necessary when I have nested loops, for example:


while ( expr1 )
{
while ( expr2 )
{
while ( expr3 )
{
goto OUT_OF_ALL_LOOPS;
}
}
}

OUT_OF_ALL_LOOPS: ;


In my own humble opinion, I think it's childish to outright ban something
when it comes to computer programming -- I feel it gives an air of
inconfidence in one's competence.
 
A

Andrew Poelstra

Ian Collins posted:




I find "goto" necessary when I have nested loops, for example:


while ( expr1 )
{
while ( expr2 )
{
while ( expr3 )
{
goto OUT_OF_ALL_LOOPS;
}
}
}

OUT_OF_ALL_LOOPS: ;


In my own humble opinion, I think it's childish to outright ban something
when it comes to computer programming -- I feel it gives an air of
inconfidence in one's competence.

This can be done in a way that is more clear (in most cases) like so:

while (expr1 && !done)
while (expr2 && !done)
while (expr3 && !done)
if (expr4)
done = 1;

This has already been mentioned in this thread. You might want to find some
archives of it. (Googling comp.lang.c "thread name" will usually get you a
lot of good archives).
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top