Here is a complete and working code that
1. Adds the numbers(input) to get a sum(output)
2. Sorts the numbers in a descending order

Can you post any suggestions on how I can improve my code :)

main.c

``````#include <stdio.h>
main (){
int sum, x, y, z,choices;
input(&x, &y, &z);
printf ("\nChoose function\n");
do{
printf("2 - Swap and Sort function \n");
printf("3 - Quit\n");
scanf ("%d", &choices);
switch (choices) {
case 1: addfunction(&x, &y, &z, &sum);
printf ("%d+%d+%d=%d.\n",x,y,z,sum);
break;
case 2: sortnswap(&x, &y, &z);
printf ("%d %d %d \n", x, y, z);
break;
case 3: printf ("\nprogram terminated!!!\n");
break;
}
}
while(choices!=3);
}``````

``````void input (int *a, int *b, int *c) {                          //Input Function

printf ("\nEnter x: ");
scanf ("%d", a);
printf ("Enter y: ");
scanf ("%d", b);
printf ("Enter z: ");
scanf ("%d", c);
}

void addfunction(int *a, int *b, int *c, int *sum) {	        //Output function
*sum=*a+*b+*c;
}

void sortnswap(int *a, int *b, int *c) {                         //Sort and Swap Function
int hi,lo,mid;
printf ("\nThe numbers sorted from descending order\n");
if (*a>=*b && *a>=*c && *b>=*c) {
hi = *c;
mid = *b;
lo = *a;
}
else if (*a>=*b && *a>=*c && *c>=*b) {
hi = *b;
mid = *c;
lo = *a;
}
else if (*b>=*a && *b>=*c && *c>=*a) {
hi = *a;
mid = *c;
lo = *b;
}
else if (*b>=*a && *b>=*c && *a>=*c) {
hi = *c;
mid = *a;
lo = *b;
}
else if (*c>=*a && *c>=*b && *b>=*a) {
hi = *a;
mid = *b;
lo = *c;
}
else if (*c>=*a && *c>=*b && *a>=*b) {
hi = *b;
mid = *a;
lo = *c;
}
*a = lo;
*b = mid;
*c = hi;
}``````

## All 3 Replies

Oh yes!

Nothing personal, but I really dislike your code:

1) It's int main(void) or int main(int argc, char *argv[]), never just main or void main.

And always with a return (usually zero) to signify success to the OS and you.

2) Your indentations are an affront to real estate on the monitor. 2 to 5 spaces is plenty per level of indentation. Your style will run you right off the row, with most real programs.

3) Your sort is an OMG! Bug city hotel. It's so long, and so error prone to work with, it's just incredible.

C should be CONCISE, (short, and to the point), not a Rube Goldberg deluxe creation. Simple and clear are secondary goals to accuracy, but important one's nevertheless.

A good sorter should be 5 to 30 lines, depending on what you choose, and what function is doing the swapping.

4) Your braces (opening and closing), should vertically line up on the same column. Yours are FAR offset.

This function, I definitely LIKE! ;)

Your code is wonderfully playful, great creativity, but never show it to someone as something like working code you wrote. If you ever need to extend or modify it, after you've been away from it for awhile, you will see what a nightmare on Elm St. it is to work with.

Oh yes!

Nothing personal, but I really dislike your code:

1) It's int main(void) or int main(int argc, char *argv[]), never just main or void main.

And always with a return (usually zero) to signify success to the OS and you.

2) Your indentations are an affront to real estate on the monitor. 2 to 5 spaces is plenty per level of indentation. Your style will run you right off the row, with most real programs.

3) Your sort is an OMG! Bug city hotel. It's so long, and so error prone to work with, it's just incredible.

C should be CONCISE, (short, and to the point), not a Rube Goldberg deluxe creation. Simple and clear are secondary goals to accuracy, but important one's nevertheless.

A good sorter should be 5 to 30 lines, depending on what you choose, and what function is doing the swapping.

4) Your braces (opening and closing), should vertically line up on the same column. Yours are FAR offset.

This function, I definitely LIKE! ;)

Your code is wonderfully playful, great creativity, but never show it to someone as something like working code you wrote. If you ever need to extend or modify it, after you've been away from it for awhile, you will see what a nightmare on Elm St. it is to work with.

Thank you for criticizing my work at least I'll become better at writing codes :idea:

C should be CONCISE, (short, and to the point), not a Rube Goldberg deluxe creation. Simple and clear are secondary goals to accuracy, but important one's nevertheless.

Concise and clear can easily be mutually exclusive. For example:

``````#include <stdio.h>

#define min(a,b) ((a) < (b) ? (a) : (b))
#define max(a,b) ((a) > (b) ? (a) : (b))
#define min3(a,b,c) (min(a, min(b, c)))
#define max3(a,b,c) (max(a, max(b, c)))
#define mid3(a,b,c) \
((a) > (b) && (a) > (c) ? \
max(b, c) : \
((a) < (b) && (b) > (c) ? max(a, c) : max(a, b)))

int main(void)
{
int x, y, z;

printf("Enter three integers: ");
fflush(stdout);

if (scanf("%d%d%d", &x, &y, &z) == 3) {
printf("%d %d %d\n", min3(x, y, z), mid3(x, y, z), max3(x, y, z));
printf("Sum: %d\n", x + y + z);
}

return 0;
}``````

This program does the same thing, but that mid3() macro is a bear even with the helper macros. This is a case where concise might be a bad thing because it hurts clarity.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, learning, and sharing knowledge.