Reputation: 17
i have been given a structure and a pointer to an array.
each index of the array is a letter of the alphabet. i need to receive a name, last name and phone number, and allocate memory to a struct (phonebook).
then, each struct needs to be accessed from the array using the last name's first letter.
if the function is called again i need to use linked list to add another contact.
i dont know how to allocate memory for a certain index of an array. when i try to do
phonebook[letter] = (Contact**)malloc(sizeof(Contact));
i keep having de reference warnings, and i cant seem to figure out how to point the address of phonebook[letter] to a structure properly.
this is what i have tried:
typedef struct Contact {
char* firstName;
char* lastName;
char* phoneNum;
struct Contact* next;
} Contact;
int main(){
Contact* phonebook[26];
addNewContact(phonebook)
}
int addNewContact(Contact** phonebook) {
char newFirstName[SIZE], newLastName[SIZE], newPhoneNum[SIZE];
int letter;
printf("Enter a contact details \
(<first name> <last name> <phone number>):\n");
scanf("%s%s%s", newFirstName, newLastName, newPhoneNum);
//get number of the letter in the alphabet
letter = newLastName[0] - 'A';
//allocate memory to pointer
Contact *current;
phonebook = (Contact**)malloc(sizeof(Contact));
if (phonebook == NULL) {
printf("The addition of the contact has failed!");
//free
exit(1);
}
current = phonebook[letter];
//check if details are being used
do {
//if the name already exists
if (phonebook[letter]->firstName == newFirstName \
&& phonebook[letter]->lastName == newLastName) {
printf("The addition of the contact has failed, \
since the contact %s %s already exists!\n", newFirstName, newLastName);
//free
return 0;
}
//if the phone number already exists
if (phonebook[letter]->phoneNum == newPhoneNum) {
printf("The addition of the contact has failed, \
since the phone number %s already exists!", newPhoneNum);
//free
return 0;
}
current = current->next;
} while (current != NULL);
//assigning
phonebook[letter]->firstName = newFirstName;
phonebook[letter]->lastName = newLastName;
phonebook[letter]->phoneNum = newPhoneNum;
return 0;
}
in addition, i havent figured out the linked list part at all, i managed before to enter the details to the structure (though im not sure if i even pointed the struct to the right place) but i dont know how to iterate after the first addition of the name. if i intialize current->next to be NULL for the first time, it will also happen the next time i call the function.
currently the code stops due do access violation but it seemed that after the first name i had error with reading the inputs that only occured after the second time.
Upvotes: 1
Views: 93
Reputation: 1922
You are actually pretty close to making it work. Probably the main problem with this code is that it has only pointers to the data, doesn't have space to store the data. You are pointing to phonebook[letter]->lastName = newLastName
, a local variable that is destroyed when the function returns. This will make your strings a dangling pointer; when you try to access this data, undefined things happen. Probably the easiest way to fix that is to make a char array
of maximum length instead of the pointers. You don't need to typedef
in most cases, and sometimes it's confusing. I would recommend that you take the typedef
out and just have,
#define SIZE 128
struct Contact {
char firstName[SIZE];
char lastName[SIZE];
char phoneNum[SIZE];
struct Contact* next;
};
Double-pointer Contact** phonebook
is valid, but a recipe for confusion. Instead of naked pointers, use struct
to encapsulate a phone book, using a list of struct Contact
.
struct Phonebook {
struct Contact *letter[26];
};
printf
is defined in stdio.h
. strcmp
is defined in string.h
. malloc
and exit
are stdlib.h
.
See What's the correct declaration of main()? C doesn't scan ahead; switch the order of the functions or have a prototype above.
Perhaps in the future you will read from a file? It's easier do if you read the input separately from the interface. Maybe split it up into a separate function. This should be static
, since it doesn't need to be published to other translation units.
static struct Contact *inputContact(void)
To allocate memory, current = malloc(sizeof *current);
. Did you have a typo using phonebook
? If you allocate it first, you can read directly into the allocated memory and not do any copying. Make sure you limit the size. (scanf
is not meant for this.)
#define STR(n) STR2(n)
#define STR2(n) #n
if(scanf("%" STR(SIZE) "s%" STR(SIZE) "s%" STR(SIZE) "s",
current->firstName, current->lastName, current->phoneNum) != 3)
exit(EXIT_FAILURE);
Then,
static int addNewContact(struct Phonebook* phonebook, struct Contact *contact)
do-while
checks that the current
is valid after you use it; you want while
.
//get number of the letter in the alphabet
int letter = contact->lastName[0] - 'A'; // might want to check that
struct Contact *current = phonebook->letter[letter];
//check if details are being used
while (current != NULL) {
In C, equality means they are the same memory location, not in general the same text. You probably want a library function strcmp
. (But are you sure your logic is correct in that it prevents the same number in appearing with the same first letter of the last name?)
if (!strcmp(current->phoneNum, contact->phoneNum))
Assigning will just overwrite the one letter that is there. You probably meant for it to be appended to the list.
contact->next = phonebook->letter[letter];
phonebook->letter[letter] = contact;
return 1;
And lastly, be sure to initialize any data that is of automatic duration.
struct Phonebook phonebook = {0}; /* Initialize null lists. */
addNewContact(&phonebook, inputContact());
--edit
This can be seen as creating a static-26-bucket separately-chained hash-table, with your hash function being hash(struct Contact *entry) { return entry->lastName[0] - 'A'; }
.
I think you are doing this, which also makes sense. Make sure that you allocate Contact
and 3 text fields, for a total of 4 malloc
s per entry. If you read the data into a temporary buffer, you can use strcpy
to transfer it to a longer-lived string that you've just allocated.
Upvotes: 1