0

I am new to C and very much interested in knowing how to approach any problem which has more than 3 or 4 functions, I always look at the output required and manipulate my code calling functions inside other functions and getting the required output. Below is my logic for finding a students record through his Id first & then Username. This code according to my professor has an excessive logic and is lacking in many ways, if someone could assist me in how should I approach any problem in C or in any other language it would be of great help for me as a beginner and yes I do write pseudo code first.

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

typedef struct{

int id;                                  //Assuming student id to be unique
int age;
char *userName;                         //Assuming student userName to be unique
char *dept;

}student;                               // Alias "student" created for struct

student* createstruct();                // All function prototype declared
student* createArray();
void addstruct(student* s2);
void searchChar(student* s2,int num);
void searchInt(student* s2,int num);

student* createstruct()                          // function createStruct() to malloc data of struct student.
{
    student *s;
    s = (student*)malloc(sizeof(student));
    s->userName = (char*)malloc(sizeof(char)*32);
    s->dept = (char*)malloc(sizeof(char)*32);
    printf("please enter id ");
    scanf("%d",&s->id);
    printf("please enter age ");
    scanf("%d",&s->age);
    printf("please enter userName ");
    scanf("%31s",s->userName);
    printf("please enter department ");
    scanf("%31s",s->dept);
    printf("\n");

    return s;
}

student* createArray()
{
    student *arr;                                    //declaration of arr poiter, type struct student
    arr = (student*)malloc(sizeof(student)*10);     // memory allocated for a size of 10
    return arr;
}

void addstruct(student *s2)                       // function for adding data to the structures in array
{
    int i,num;
    student* s1;
    printf("please enter the number of records to add:");
    scanf("%d",&num);
    printf("\n");

    if(num>0 && num<11)
    {
      for(i=0;i<num;i++)                    // if user want to enter 5 records loop will only run 5 times
       {
         s1 = createstruct();
         s2[i].id = s1->id;                 // traversing each element of array and filling in struct data
         s2[i].age = s1->age;
         s2[i].userName = s1->userName;
         s2[i].dept= s1->dept;
       }
    }
    else if(num>10)                         // if user enters more than 10
    {
      for(i=0;i<10;i++)                     // loop will still run only 10 times
        {
         s1 = createstruct();
         s2[i].id = s1->id;
         s2[i].age = s1->age;
         s2[i].userName = s1->userName;
         s2[i].dept = s1->dept;
        }

       printf("Array is full");            // Array is full after taking 10 records
       printf("\n");
    }

searchInt(s2,num);                        // Calling searchInt() function to search for an integer in records
searchChar(s2,num);                       // Calling searchChar() function to search for a string in records
free(s1);
free(s2);
}

void searchChar(student* s2,int num)           // function for searching a string in records of structure
{
    char *c;
    int i;
    c = (char*)malloc(sizeof(char)*32);
    printf("please enter userName to search ");
    scanf("%31s",c);
    printf("\n");

  for (i=0;i<num;i++)                             //num is the number of struct records entered by user
    {
      if ((strcmp(s2[i].userName,c)==0))          //using strcmp for comparing strings
      {
       printf("struct variables are %d, %d, %s, %s\n", s2[i].id,s2[i].age,s2[i].userName,s2[i].dept);
       break;
      }
      else if(i == num-1)
      {
          printf("nothing in userName matches: <%s>\n",c);
          break;
      }
    }
}

void searchInt(student* s2,int num)                 //searchs for an integer and prints the entire structure
{
    int i,z;
    printf("please enter id to search ");
    scanf("%d",&z);
    printf("\n");

  for (i=0;i<num;i++)
    {
      if (s2[i].id == z)
      {
          printf("struct variables are %d, %d, %s, %s\n\n", s2[i].id,s2[i].age,s2[i].userName,s2[i].dept);
          break;
      }
      else if(i == num-1)
      {
          printf("nothing in id matches: <%d>\n\n",z);
          break;
      }
    }
}

int main(void)
{
    student *s2;
    s2 = createArray();
    addstruct(s2);
    return 0;
}
David Robinson
  • 77,383
  • 16
  • 167
  • 187
Vbp
  • 1,912
  • 1
  • 22
  • 30
  • 2
    what does it mean to "frame code"? – Mikhail Oct 25 '12 at 03:14
  • Corrected it, I just meant approach and how to go ahead with any problem – Vbp Oct 25 '12 at 03:23
  • 1
    See the answers on these questions: http://stackoverflow.com/questions/4842817/how-do-i-learn-to-write-efficient-and-maintainable-c-code http://stackoverflow.com/questions/1711/what-is-the-single-most-influential-book-every-programmer-should-read – max Oct 25 '12 at 03:27
  • My advice is: Don't optimize until you can see that there is a need. To see that, you need to _profile_ and _measure_. Many source-code optimizations make the source less readable and more prone to errors. – Some programmer dude Oct 25 '12 at 07:22

1 Answers1

0

I'm not going to go into optimizing, because if you wanted better theoretical performance you would probably go with different data structures, such as ordered arrays/lists, trees, hash tables or some kind of indexing... None of that is relevant in this case, because you have a simple program dealing with a small amount of data.

But I am going to tell you about the "excessive logic" your professor mentioned, taking your searchInt function as an example:

for (i=0;i<num;i++)
{
  if (s2[i].id == z)
  {
      printf("struct variables are %d, %d, %s, %s\n\n", s2[i].id,s2[i].age,s2[i].userName,s2[i].dept);
      break;
  }
  else if(i == num-1)
  {
      printf("nothing in id matches: <%d>\n\n",z);
      break;
  }
}

The thing here is that every time around the loop you're testing to see if you're at the last element in the loop. But the loop already does that. So you're doing it twice, and to make it worse, you're doing a subtraction (which may or may not be optimized into a register by the compiler).

What you would normally do is something like this:

int i;
student *s = NULL;

for( i = 0; i < num; i++ )
{
    if( s2[i].id == z ) {
        s = &s2[i];
        break;
    }
}

if( s != NULL ) {
    printf( "struct variables are %d, %d, %s, %s\n\n",
            s->id, s->age, s->userName, s->dept );
} else {
    printf("nothing in id matches: <%d>\n\n",z);
}

See that you only need to have some way of knowing that the loop found something. You wait for the loop to finish before you test whether it found something.

In this case I used a pointer to indicate success, because I could then use the pointer to access the relevant record without having to index back into the array and clutter the code. You won't always use pointers.

Sometimes you set a flag, sometimes you store the array index, sometimes you just return from the function (and if the loop falls through you know it didn't find anything).

Programming is about making sensible choices for the problem you are solving. Only optimize when you need to, don't over-complicate a problem, and always try to write code that is easy to read/understand.

paddy
  • 60,864
  • 6
  • 61
  • 103