Student sorting based on criteria

This code takes id, name and marks of 3 subjects for 5 students from user base, calculates the total of 3 subjects per student, and then takes a criteria level with which to sort; level 1 sorting does sorting based on 1 criteria only, while level 2 will sort based on criteria 1 and if similar occurs (like if select level 1 criteria marks and if marks of two student are same) sorting of that pair will be done based on criteria 2, if either user have selected level 2 and criteria 2.

```
#include
#include
#include

struct student
{
int id;
int sub[3];
int total;
char name;
}s[5];
void sort1(struct student *[],int,int,int);
main()
{ int i,j,ch,c1,c2,t;
char choice;
for(i=0;iname,&p[i+j]->name)>0))
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
else if((t==1) &&(cri2==2)&& (strcmp(&p[i]->name,&p[i+j]->name)==0))
{
if(p[i]->idid)
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
}
else if((t==1)&&(cri2==3)&&(strcmp(&p[i]->name,&p[i+j]->name)==0))
{
if(p[i]->totaltotal)
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
}
else if((t==0)&&(cri1==1) && (strcmp(&p[i]->name,&p[i+j]->name)>0))
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
else if((t==0) &&(cri1==2)&& (strcmp(&p[i]->name,&p[i+j]->name)>0))
{
if(p[i]->idid)
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
}
else if((t==0)&&(cri1==3)&&(strcmp(&p[i]->name,&p[i+j]->name)>0))
{
if(p[i]->totaltotal)
{
pt=p[i];
p[i]=p[i+j];
p[i+j]=pt;
}
}

}
}
for(i=0;iid,p[i]->name,p[i]->sub[0],p[i]->sub[1],p[i]->sub[

indentation

The most obvious weirdness about your code is the inconsistent indentation. Indeed I was convinced that there was a missing } at the end until I scanned the whole thing much more carefully. Indent everything within a function one level and add additional levels at each new nested statement. (user1118321 has generously provided a version with corrected indentation)

long lines

Very long lines are hard to read. This one, requires me to scroll to read the whole thing. So I can't even see the whole function call all-at-once.

printf("Id\tName\tS1\tS2\tS3\tTotal\n\n%d\t%s\%d\t%d\t%d\t%d\n",p[i]->id,p[i]->name,p[i]->sub[0],p[i]->sub[1],p[i]->sub[2],p[i]->total);

You can fold this nicely over multiple lines.

printf("Id\tName\tS1\tS2\tS3\tTotal\n\n%d\t%s\%d\t%d\t%d\t%d\n",
           p[i]->id,p[i]->name,
           p[i]->sub[0],p[i]->sub[1],p[i]->sub[2],
           p[i]->total);

You can even break-up long strings and let the compiler concatenate them for you.

printf("Id\tName\t" "S1\tS2\tS3\t" "Total\n\n"
           "%d\t%s\t"   "%d\t%d\t%d\t" "%d\n",
           p[i]->id,p[i]->name,
           p[i]->sub[0],p[i]->sub[1],p[i]->sub[2],
           p[i]->total);

Incidentally, I found a missing "t" in there while doing this.

index origin

For output you add 1 to the index, but for input you do not subtract 1. This is bizarre. It would be more natural in a C program to use 0 throughout. The first element is element 0. If you need to, you can document it for your users or even announce it at program startup.

interactive/batch

A more useful program would be one that operates on data-files and accepts command-line arguments as instructions. This saves you lots of typing as you test your program. You can type all the data just once and run your program many times. You can test different options by writing a shell-script or batch-file which runs the program different ways.

You could define a text-representation of your data. A common way is to use : as a separator

Student,Lastname:25:37:44:55
Student,Noname:0:0:0:99
Studentka,Foxy:101:110:150:1000

Then your program could accept the filename and a code or option as an argument.

int main(int argc, char **argv){
    if (argc < 3) {
        printf("too few arguments\n"
               "usage: %s filename option\n"
               "where option is 0 1 2 or 3\n");
    }

    struct student *data = loadfile(argv[1]);
    perform_sort(data, strtol(argv[2],NULL,10));
    print_data(data);
}

It would be good to separate the different parts into different functions, but this is easier to see when you try to make it work with files. You could also accept - as a special file-name that means stdin, and still run your program
interactively. You can trigger an End-Of-File condition with ctrl-D (Unix, OSX) or ctrl-Z (Windows) to tell your program where the data ends. This is a common behavior for command-line utilities.

dynamically resizing arrays

In order to approach what I'm describing here, it would be good to build the student database dynamically.

student *data = NULL;
int data_n = 0;

data = malloc((data_n=10) * sizeof *data);

A couple of things to note. I assign the size and allocate the array in the same statement. I don't know if everybody does this, but it helps me make sure that the two variables always change together. It would be good to check the result of malloc for a NULL and do something sensible such as exit(1), but for very simple programs you can usually get away with just assuming malloc will return the memory. This is a fragile behavior and the program is not correct per se, but memory-management is a complicated subject to learn, so it's also good to keep things simple until there is a need to make them complicated.

So what happens if you're reading line number 11? Every time the loadfile function will read a line, it will first check that i < data_n and if not,
then it must first

data = realloc(data, (data_n *= 2) * sizeof *data);

to make more room. When you finish reading data, you should make sure there is still some room (i < data_n) and then add a sentinel record to indicate the end of data for the other functions. This could be a student with the name "\0" perhaps.

char ... pointer?

char name;

//...

scanf("%s",&s[i].name);

Unless the name is 0 characters long, that's not enough memory for a name. You could set a maximum size, which is perhaps the easiest way.

char name[100];

Or you could use a pointer to a string, but you must allocate memory for it, and for that it is still easiest to use a fixed-size buffer and then use the POSIX function strdup to make a dynamic copy of the string.

StackExchange Code Review Q#121639, answer score: 7