> I've been learning primarly from a book written by Dan Gookin and he uses global variables when using variables across multiple functions.
You're learning a bunch of bad habits.
The way you solve the problem is by returning results as well as passing parameters.
> I'd prefer to focus my efforts on fixing the existing code rather than essentially starting from scratch.
Learning to throw away bad code is also a skill.
The biggest mistake you're making is copy/pasting the same code 4 times over in your add_record.
FWIW, no globals.
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
struct record {
char c_detail1[20];
int i_value;
struct record *next;
};
typedef struct record record_t;
struct node {
char key[15];
record_t *record_list;
struct node *next;
};
typedef struct node node_t;
// NO GLOBALS HERE!!!
// ------------------
// A separate function to traverse each list.
// Lesson: Don't try to do too much in one function
// Lesson: Each function should do one thing, and do it well.
void print_record_list(const record_t *reclist) {
while (reclist != NULL) {
printf("Detail: %s\n",reclist->c_detail1);
printf("Value: %d\n",reclist->i_value);
printf("\n");
reclist = reclist->next;
}
}
void print_key_list(const node_t *keylist) {
while (keylist != NULL) {
printf("Key: %s\n", keylist->key);
print_record_list(keylist->record_list);
keylist = keylist->next;
}
}
node_t *create_node(const char *key) {
// prepare a new node
node_t *new = malloc(sizeof(*new));
strcpy(new->key,key);
new->record_list = NULL;
new->next = NULL;
return new;
}
// By returning the list, we can write self-starting lists
// using nothing more than a local variable initialised to NULL.
// See how it's called in main.
node_t *add_key(node_t *keylist, const char *key) {
// prepare a new node
node_t *new = create_node(key);
if (keylist == NULL) {
keylist = new;
} else {
node_t *tail = keylist;
while (tail->next != NULL) {
tail = tail->next;
}
tail->next = new;
}
return keylist;
} /* add_key */
record_t *create_record(const record_t *data) {
record_t *new = malloc(sizeof(*new));
strcpy(new->c_detail1, data->c_detail1);
new->i_value = data->i_value;
new->next = NULL;
return new;
}
void add_record(node_t *keylist, const char *key, const record_t *data) {
// prepare new node
record_t *new = create_record(data);
// find the key
node_t *current = keylist;
while( current != NULL && strcmp(current->key,key) != 0 ) {
current = current->next;
}
if( current != NULL ) {
// found the key, now insert the record
// Lesson: Compare this if/else code with that in add_key
if (current->record_list == NULL) {
current->record_list = new;
} else {
record_t *tail = current->record_list;
while (tail->next != NULL) {
tail = tail->next;
}
tail->next = new;
}
}
} /* add_record */
int main()
{
node_t *node_list = NULL;
node_list = add_key(node_list,"Item 1");
node_list = add_key(node_list,"Item 2");
node_list = add_key(node_list,"Item 3");
print_key_list(node_list);
record_t *record = malloc(sizeof(*record));
strcpy(record->c_detail1,"some string");
record->i_value = 100;
add_record(node_list,"Item 1",record);
print_key_list(node_list);
return(0);
}
Originally Posted by Bjarne Stroustrup (2000-10-14)
