Array of pointers implementation in C

Question

Hii ,

I have been trying to write a program... We have a structure which has a rank field and the name field.The pointer to this structure is stored in an array of fixed size. I have implemented it as follows and i have certain problems... The code i have written is :

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

typedef struct 
{
 int rank;
 char *name;
}node;

int insert(node **a , char name[] , int *rank)
 {
 if(*rank >= 5)
  {
   printf("\n Overflow ");
   return 0;
  } 
  (*rank)++;
  node *new = (node *)malloc(sizeof(node));
  new->name = name;
  new->rank = *rank;
  a[*rank] = new;

  return 0;
 } 

int delete(node **a , int *rank)
 {
  int i = *rank;
  if(*rank<0)
   {
    printf("\n No elements");
    return 0;
   }
   printf("\n Deleting %d , %s ",((a[*rank]))->rank,((a[*rank]))->name);
   printf("\n Reordering the elements ");
   while(i<5)
    {
     a[i] = a[i+1];
    }  
  return 0;
 }

 int display(node **a , int rank)
  {
   while(rank>0 && (a[rank])>0)
    {
     printf(" rank = %d    name = %s \n",((a[rank])->rank),((a[rank])->name));
     rank--;
    }            
    return 0;
  }

int main()
 {
  node *a[5] = {NULL};
  char ch = 'y';
  int choice,rank = -1;
  char name[10];
  while(ch!='n' || ch!= 'N')
   {
    printf("\n Enter 1 to insert , 2 to delete , 3 to display and 4 to exit \n");
    scanf("%d",&choice);
    switch(choice)
     {
      case 1:
        printf("\n Enter name to insert");
        gets(name);
        insert(a,name,&rank);
        break;
      case 2:
        printf("\n Enter rank to delete ");
        scanf("%d",&rank);
        delete(a,&rank);
        break;
      case 3:
        display(a,rank);
        break;
      case 4:
        exit(0);
      default:
        printf("\n Invalid choice...please enter again ");
        break;
     } 
    ch = getchar();
  }
 return 0;
 } 

First thing is the system automatically takes the choice except for the first time...(i couldn't find the fault there...) and i am a bit confused about this pointer stuff...Please see if its alright...Any corrections are welcome and please give me some explanation as to why it is wrong and how we shd do it...

Thank You

Answer 1

First of all, all your functions always return 0 -- even for an error condition. Life would be so much easier, if you passed rank in as an int, and returned it's new value.

rank = insert(a, name, rank); 
/* : */
/* : */
int insert(node **a , char name[] , int rank)  
{  
 if(rank >= 5)  
 {  
   printf("\n Overflow ");  
   return 0;  
 }   
 rank++;  
 node *new = (node *)malloc(sizeof(node));  
 new->name = name;  
 new->rank = rank;  
 a[rank] = new;  
 return rank;  
}

It's been many years since I last used scanf, but as I recall, you must account for every character in the stream, meaning,"Don't forget the Enter".

scanf("%d\n",&choice);  

Also with gets(name);, if you type more tha 9 characters, you are quite screwed, as it will overwrite the stack of your program.

UPDATE: Also, you have two ways to exit this program, except one will never work. You could choose option "4" which will call exit(0). Alternately, at the end of each command, you wait for a character before stepping over. It appears you want to be able to enter "N" ther and exit, except that won't work:

while(ch!='n' || ch!= 'N') 

for that to evaluate to false, ch must be both "n" & "N" at the same time. You really want

while(ch!='n' && ch!= 'N') 

UPDATE2: I just noticed the biggest problem in you code. name everywhere in your code only ever points to the single array defined in main(). Everytime you enter a new name, it overwrites that array, and since every node points to that one array, the name changes everywhere. You need to make a copy. in insert():

node *new = (node *)malloc(sizeof(node));     
new->name = strdup(name);    // use malloc internally.

Then in delete() you'll have to free that memory (speaking of which, you need to free node there too...)

printf("\n Deleting %d , %s ",((a[*rank]))->rank,((a[*rank]))->name);       
free(a[*rank]->name);
free(a[*rank]);
printf("\n Reordering the elements ");

Remember, Whenever you call malloc, you will eventually have to call free.

Answer 2

I'm confused as to the use of the "rank" variable. Main uses it as an index to the last node in the array, and the nodes use it as a ranking. Adding nodes increases it, but deleting nodes doesn't decrease it.

In the very least, I'd suggest separate the index variable from the ranking variable to make the logic easier to follow.

Personally, I'd write a structure to encapsulate the array with its own index tracking and add/delete functions. That way Main is free to read in user options and manipulate rank of new nodes without worrying about data structure details.

Answer 3

 while(ch!='n' || ch!= 'N')
   {
    printf("\n Enter 1 to insert , 2 to delete , 3 to display and 4 to exit \n");
    scanf("%d",&choice); getchar();
    .
    .
    .
    //ch = getchar();
  }

Using getchar() along with scanf() causes this problem. Since '\n' after reading a character into 'ch' goes as an input to scanf. One way to resolve your problem is read that '\n' with an extra getchar() before it is read by gets(). Also you should modify while loop in delete.