Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to make sorting function simpler (or shorter)

Tags:

c

sorting

I am writing a code that reads a binary file, reads each struct field separately, sorts the output, then writes the output to a binary file.

Below is my code.

#define MAX_HAT 11
#include<stdio.h>
#include<stdlib.h>

//struct variables
struct test
{
   short int land;
   double experience;
   short int boys;
   int angle;
   float industry;
   unsigned short thread;
   unsigned char shoe;
   double kitty;
   unsigned long price;
   short int father;
   char room;
   int attraction;
   int cake;
   int foot;
   char hat[MAX_HAT];
   char nest;
   double bean;
};

int compare (const void * a, const void * b) 
   {
   
   struct test** a1 = (struct test**) a;
   struct test** b1 = (struct test**) b;

   //experience(asc)
   if ((*a1)->experience > (*b1)->experience)
   {
       return 1;
   }
   if ((*a1)->experience < (*b1)->experience)
   {
       return -1;
   }
   //land(asc)
   if ((*a1)->land > (*b1)->land)
   {
       return 1;
   }
   if ((*a1)->land < (*b1)->land)
   {
       return -1;
   }
   //nest(desc)
   if ((*a1)->nest > (*b1)->nest)
   {
       return -1;
   }
   if ((*a1)->nest < (*b1)->nest)
   {
       return 1;
   }
   //shoe(asc)
   if ((*a1)->shoe > (*b1)->shoe)
   {
       return 1;
   }
   if ((*a1)->shoe < (*b1)->shoe)
   {
       return -1;
   }
   //hat(desc)
   if (strncmp((*b1) -> hat, (*a1) -> hat, MAX_HAT) != 0)
   {
       return strncmp((*b1) -> hat, (*a1) -> hat, MAX_HAT);
   }
   //boys(asc)
   if ((*a1)->boys > (*b1)->boys)
   {
       return 1;
   }
   if ((*a1)->boys < (*b1)->boys)
   {
       return -1;
   }
   //cake(desc)
   if ((*a1)->cake > (*b1)->cake)
   {
       return -1;
   }
   if ((*a1)->cake < (*b1)->cake)
   {
       return 1;
   }
   //price(desc)
   if ((*a1)->price > (*b1)->price)
   {
       return -1;
   }
   if ((*a1)->price < (*b1)->price)
   {
       return 1;
   }
   //bean(desc)
   if ((*a1)->bean > (*b1)->bean)
   {
       return -1;
   }
   if ((*a1)->bean < (*b1)->bean)
   {
       return 1;
   }
   //room(desc)
   if ((*a1)->room > (*b1)->room)
   {
       return -1;
   }
   if ((*a1)->room < (*b1)->room)
   {
       return 1;
   }
   //father(desc)
   if ((*a1)->father > (*b1)->father)
   {
       return -1;
   }
   if ((*a1)->father < (*b1)->father)
   {
       return 1;
   }
   //industry(desc)
   if ((*a1)->industry > (*b1)->industry)
   {
       return -1;
   }
   if ((*a1)->industry < (*b1)->industry)
   {
       return 1;
   }
   //foot(desc)
   if ((*a1)->foot > (*b1)->foot)
   {
       return -1;
   }
   if ((*a1)->foot < (*b1)->foot)
   {
       return 1;
   }
   //angle(asc)
   if ((*a1)->angle > (*b1)->angle)
   {
       return 1;
   }
   if ((*a1)->angle < (*b1)->angle)
   {
       return -1;
   }
   //kitty(asc)
   if ((*a1)->kitty > (*b1)->kitty)
   {
       return 1;
   }
   if ((*a1)->kitty < (*b1)->kitty)
   {
       return -1;
   }
   //attraction(asc)
   if ((*a1)->attraction > (*b1)->attraction)
   {
       return 1;
   }
   if ((*a1)->attraction < (*b1)->attraction)
   {
       return -1;
   }
   //thread(desc)
   if ((*a1)->thread > (*b1)->thread)
   {
       return -1;
   }
   if ((*a1)->thread < (*b1)->thread)
   {
       return 1;
   }
       return 0;
}

int main(int argc, char **argv)
{
   int size = 8;
   int count = 0;
   int i;

   struct test *t1;
   t1 = (struct test*) malloc (size * sizeof(struct test));

   FILE *fp1;
   FILE *fp2;
   
   if (!t1)
   {
       fprintf(stderr, "Could not allocate memory");
       exit(-2) ;
   }

   fp1 = fopen(argv[1], "rb");
   fp2 = fopen(argv[2], "wb");

   if (argc != 3)
   {
       fprintf(stderr, "Incorrect file names");
       exit(1);
   }

   if (!fp2)
   {
       fprintf(stderr, "Could not open file %s\n", argv[2]);
       exit(-3);
   }

   if (!fp1)
   {
       fprintf(stderr, "Could not open file %s\n", argv[1]);
       exit(-3);
   }
   
   while (!feof(fp1))
   {
       if (count >= size)
       {
           size = size * 2;
           t1 = realloc(t1, sizeof(struct test) * size);
       }
   
       fread(&t1[count].land, sizeof(t1[count].land), 1, fp1);
       fread(&t1[count].experience, sizeof(t1[count].experience), 1, fp1);
       fread(&t1[count].boys, sizeof(t1[count].boys), 1, fp1);
       fread(&t1[count].angle, sizeof(t1[count].angle), 1, fp1);
       fread(&t1[count].industry, sizeof(t1[count].industry), 1, fp1);
       fread(&t1[count].thread, sizeof(t1[count].thread), 1, fp1);
       fread(&t1[count].shoe, sizeof(t1[count].shoe), 1, fp1);
       fread(&t1[count].kitty, sizeof(t1[count].kitty), 1, fp1);
       fread(&t1[count].price, sizeof(t1[count].price), 1, fp1);
       fread(&t1[count].father, sizeof(t1[count].father), 1, fp1);
       fread(&t1[count].room, sizeof(t1[count].room), 1, fp1);
       fread(&t1[count].attraction, sizeof(t1[count].attraction), 1, fp1);
       fread(&t1[count].cake, sizeof(t1[count].cake), 1, fp1);
       fread(&t1[count].foot, sizeof(t1[count].foot), 1, fp1);
       fread(&t1[count].hat, sizeof(t1[count].hat), 1, fp1);
       fread(&t1[count].nest, sizeof(t1[count].nest), 1, fp1);
       fread(&t1[count].bean, sizeof(t1[count].bean), 1, fp1);
       
       count++;
   }
   
   struct test** t2 = (struct test**) malloc (size * 8);
   for (i = 0; i < count; i++)
   {
       t2[i] = &t1[i];
   }
   
   qsort(t2, count - 1, 8, compare);

   for (i = 0; i < count -1; i++)
   {
       fwrite(&t2[i]->land, sizeof(t2[i]->land), 1, fp2);
       fwrite(&t2[i]->experience, sizeof(t2[i]->experience), 1, fp2);
       fwrite(&t2[i]->boys, sizeof(t2[i]->boys), 1, fp2);
       fwrite(&t2[i]->angle, sizeof(t2[i]->angle), 1, fp2);
       fwrite(&t2[i]->industry, sizeof(t2[i]->industry), 1, fp2);
       fwrite(&t2[i]->thread, sizeof(t2[i]->thread), 1, fp2);
       fwrite(&t2[i]->shoe, sizeof(t2[i]->shoe), 1, fp2);
       fwrite(&t2[i]->kitty, sizeof(t2[i]->kitty), 1, fp2);
       fwrite(&t2[i]->price, sizeof(t2[i]->price), 1, fp2);
       fwrite(&t2[i]->father, sizeof(t2[i]->father), 1, fp2);
       fwrite(&t2[i]->room, sizeof(t2[i]->room), 1, fp2);
       fwrite(&t2[i]->attraction, sizeof(t2[i]->attraction), 1, fp2);
       fwrite(&t2[i]->cake, sizeof(t2[i]->cake), 1, fp2);
       fwrite(&t2[i]->foot, sizeof(t2[i]->foot), 1, fp2);
       fwrite(&t2[i]->hat, sizeof(t2[i]->hat), 1, fp2);
       fwrite(&t2[i]->nest, sizeof(t2[i]->nest), 1, fp2);
       fwrite(&t2[i]->bean, sizeof(t2[i]->bean), 1, fp2);
       
       //Print out the outputs
   printf("%d, %f, %i, %i, %f, %d, %d, %f, %li, %d, %d, %d, %d, %x, %s, %c, %f\n", t2[i]->land, t2[i]->experience, t2[i]->boys, t2[i]->angle, t2[i]->industry, t2[i]->thread, t2[i]->shoe, t2[i]->kitty, t2[i]->price, t2[i]->father, t2[i]->room, t2[i]->attraction, t2[i]->cake, t2[i]->foot, t2[i]->hat, t2[i]->nest, t2[i]->bean);
   }

   fclose(fp1);
   fclose(fp2);

   free(t1);
   free(t2);

   return 0;
}

As you can see in the beginning of my code, my int compare function is very long. I would like to make it simpler(or shorter) if possible. Sorting order has following condition.

Sorting order (in order of precedence):
 
experience in ascending order. 
land in ascending order. 
nest in descending order. 
shoe in ascending order. 
hat in descending order. 
boys in ascending order. 
cake in descending order. 
price in descending order. 
bean in descending order. 
room in descending order. 
father in descending order. 
industry in descending order. 
foot in descending order. 
angle in ascending order. 
kitty in ascending order. 
attraction in ascending order. 
thread in descending order. 

Would you please help me out how it can be done. Cheers.

like image 930
staypersistent Avatar asked Mar 09 '26 10:03

staypersistent


1 Answers

As you can see in the beginning of my code, my int compare function is way too long. I would like to make it simpler if possible.

This sounds as if you consider shorter code to be simpler than longer code. That's not correct. Often it's actually the opposite. Short code often requires complex solutions while writing code which is easy to read and understand leads to more code lines.

Don't use complex, hard to read/understand solutions to reduce the number of code lines. Keep things simple - even if it requires extra typing.

Using more or less complicated macro solution is a bad idea. Use plain simple C so that your code is easy to understand.

Here is one way to reduce the number of code lines while still keeping stuff in simple C code.

Function based solution

int compare_int(long long int a, long long int b)
{
    if (a > b) return 1;
    if (a < b) return -1;
    return 0;
}

int compare_unsigned(long long unsigned a, long long unsigned b)
{
    if (a > b) return 1;
    if (a < b) return -1;
    return 0;
}

int compare_double(double a, double unsigned b)
{
    if (a > b) return 1;
    if (a < b) return -1;
    return 0;
}

Now you can use these function for the fields like:

struct test** a1 = (struct test**) a;
struct test** b1 = (struct test**) b;
struct test* pa = *a1;
struct test* pb = *b1;

int cmp;
if ((cmp = compare_int(pa->land, pb->land))) return cmp;
if ((cmp = compare_double(pa->experience, pb->experience))) return cmp;
if ((cmp = compare_int(pa->boys, pb->boys))) return cmp;
if ((cmp = compare_int(pa->angle, pb->angle))) return cmp;
if ((cmp = compare_int(pa->industry, pb->industry))) return cmp;
...
return 0;

Notice how the compare_xxx function parameter is written with the largest possible type. By doing that they can be safely used for all "smaller" types. This may however have a little performance penalty. Likewise for function call but here you can try with inline

BTW:

char hat[MAX_HAT];

can't be compared using > (etc). You probably want to use strcmp.

Macro based solution

If you really want to use a macro then you could do like:

// Kind of ugly macro but not extremely complex... so perhaps okay
#define CMP_NUMBER(a, b) (((a) > (b)) ? (1) : (((a) < (b)) ? (-1) : (0)))


int cmp;
if ((cmp = CMP_NUMBER(pa->land, pb->land))) return cmp;
if ((cmp = CMP_NUMBER(pa->experience, pb->experience))) return cmp;
if ((cmp = CMP_NUMBER(pa->boys, pb->boys))) return cmp;
if ((cmp = CMP_NUMBER(pa->angle, pb->angle))) return cmp;
if ((cmp = CMP_NUMBER(pa->industry, pb->industry))) return cmp;
...
return 0;

I prefer the function based solution but this kind of macro solution is also pretty okay as it is pretty simple.

like image 114
Support Ukraine Avatar answered Mar 12 '26 03:03

Support Ukraine