Code:
void PrintCells(){
Cell *pLocal;
int iCount = 0;
pLocal = g_AtoDHashCell;
while(pLocal->pnext != NULL){
printf("Value of Cell #%d:\n",++iCount);
printf(" %s, %s\n",pLocal->lname,pLocal->fname);
printf(" %s, %s\n\n",pLocal->streetname,pLocal->cityname);
pLocal = pLocal->pnext;
}
}
Most of the time C code is indented such that global entities such as function definitions are not indented.
Code:
int x;
void f(void);
void f(void) {
int x;
}
The way you have it is fine, though.
This code could use some improving.
Code:
int HashCode(char lastname[]){
int HashKey;
char firstltrlast[STRINGBUF];
strncpy(firstltrlast, lastname,1);
firstltrlast[1] = '\0';
if(strpbrk(firstltrlast,"ABCD")!=NULL){
HashKey=1;
}
if(strpbrk(firstltrlast,"EFGH")!=NULL){
HashKey=2;
}
if(strpbrk(firstltrlast,"IJKL")!=NULL){
HashKey=3;
}
if(strpbrk(firstltrlast,"MNOP")!=NULL){
HashKey=4;
}
if(strpbrk(firstltrlast,"QRST")!=NULL){
HashKey=5;
}
if(strpbrk(firstltrlast,"UVWXYZ")!=NULL){
HashKey=6;
}
return (HashKey);
}
HashKey isn't initialized, so if none of the ifs is true the function returns an undefined value. I would initialize it to zero. Also, those ifs could be else-ifs.
That could also be the source of your problem. Because those ifs aren't else-ifs, it doesn't matter if there's an 'M' in the string; if there's a 'Z' as well, the function will return 6 (indicating 'Z').
The strncpy could easily be replaced with
Code:
firstltrlast[0] = lastname[0];
The only reason firstltrlast needs to exist at all is because strpbrk needs a string argument. If you use strchr() instead, you could replace this
Code:
if(strpbrk(firstltrlast,"ABCD")!=NULL){
with this
Code:
if(strchr("ABCD", lastname[0])!=NULL){
and get rid of firstltrlast completely.
Now, this switch statement is also pretty pointless.
Code:
void AddCell(char lastname[],char firstname[],char street[],char city[]){
switch(HashCode(lastname)){
case 1:
AssignCell(lastname,firstname,street,city,1);
break;
case 2:
AssignCell(lastname,firstname,street,city,2);
break;
case 3:
AssignCell(lastname,firstname,street,city,3);
break;
case 4:
AssignCell(lastname,firstname,street,city,4);
break;
case 5:
AssignCell(lastname,firstname,street,city,5);
break;
case 6:
AssignCell(lastname,firstname,street,city,6);
break;
default:
printf("Unable to hash.\n");
}
return;
}
You can replace the whole thing with
Code:
int hashnum = HashCode(lastname);
if(hashnum >= 1 && hashnum <= 6) {
AssignCell(lastname,firstname,street,city,hashnum);
}
else puts("Unable to hash.");
unless, for some reason, you need to use a switch statement, in which case you can use the fall-through behaviour
Code:
switch((hasnum = HashCode(lastname))){
case 1:
case 2:
case 3:
case 4:
case 5:
case 6:
AssignCell(lastname,firstname,street,city,hashnum);
break;
default:
printf("Unable to hash.\n");
}
unless you can't use a variable for some reason, in which case that's a stupid assignment.
Also, you might want to check the return value of sscanf() to see if it encountered any errors.