Median of numbers stored in array

Question

Problem is to calculate median of numbers. My idea is :

1. Fill array of integer with numbers using Arraymake() function
2. copy the address into a pointer created in main() function
3. pass that array to Median() function and calculate that median

Code:

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

    int* Arraymake(size_t);//Function for creating array of numbers.
    void  Median(size_t ,int* );//Function for calculate the median.

    int main(int argc, char **argv)
    {
        size_t count;//Size of the list of numbers.
        puts("Size of the list:");
        scanf("%Iu",&count);
        int *ArResult=Arraymake(count);//Result of the first function
        Median(count,ArResult);
        free(ArResult);
        return 0;
    }

    int* Arraymake(size_t ln)   //ln~lenght
    {
        int* Array=(int*) malloc(ln*sizeof(int));
        int number ,i; //numbers filling the array

        puts("Please input numbers.\n");

        for(i=0;i<(ln);i++)
            {
                scanf("%i", &number);
                Array[i]=number;
            }
            return Array;
    }

    void  Median(size_t ln,int* Array)
    {
        float median;

        if((ln)%2==0)
            {
                    median= (Array[(ln)/2]+Array[(ln/2 )-1] )/2;
                    printf("%f",median);


            }
        else 
            {
                    median= Array[((ln-1)/2)];
                    printf("%f",median);
            }
}

I want to find a way to optimize my variables and make my code more self-documented? How can I improve my code?

Answer 1

1. Arraymake:

• Change the name to MakeArray as it makes more sense or name it MakeArrayFromInput to make it more descriptive to what it actually does
• Name the parameter length, as in : int* MakeArray(size_t length) - this is descriptive and you don't have to put the comment explaining what it does
• Name local variables in camelCase, so array instead of Array
• Matter of preference but consider declaring array as int *array instead of int* array - you won't forget that the variable itself is a pointer and you have to declare additional pointers in the same statement with *s - int *array, *anotherPointer;
• int number ,i; //numbers filling the array - comment is redundant and you don't have to declare i here, you can do it in the for - speaking of which:
• for(i=0;i<(ln);i++) can become for(int i = 0; i < length; i++) - parentheses over ln are redundant, you should use better spacing to make it more readable.

---

2. Median:

• The same story with the parameter and naming, also pass the array first, then the length : void Median(int *array, size_t length)
• You don't have to declare the median variable before the if
• Parentheses in the if are redundant : if(length % 2 == 0)
• The calculation of the median in the case of an even number of elements is flawed - you do an integer division as both operands of / are ints - you'd have to make at least one of them a float e.g. like this median= (Array[(ln)/2]+Array[(ln/2 )-1] )/2.0; - see the 2.0 - this is a double literal, a floating-point number, not an int
• You can shorten the calculation a bit by changing array[(length - 1) / 2]; to array[length / 2]; in the case of an odd number - the integer division will truncate the result into what you need
• The function should do one thing - you're calculating the median and printing it in the same function. Consider making the function Median return the value of the median and print it somewhere else

The function then should look more or less like this :

float Median(int *array, size_t length)
{
    if(length % 2 == 0)
    {
        float median = (array[length / 2] + array[length / 2 - 1]) / 2.0;
        return median;
    }
    else 
    {
        float median = array[length / 2];
        return median;

        //or just simply
        return array[length / 2];
    }
}

Something more complex, the function can be greatly simplified by using the ternary operator ?: :

float Median(int *array, size_t length)
{
    return length % 2 == 0 ? 
           (array[length / 2] + array[length / 2 - 1]) / 2.0 : 
           array[length / 2];
}

However, this is just trivia and you should aim to make your code as readable as possible

---

3. main

• I think that all of the comments are redundant here, everything is self-documenting
• Naming : ArResult should be something descriptive and simple - the most simple way would be to name it just array
• Spacing in function parameters e.g. scanf("%Iu", &count); instead of scanf("%Iu",&count);

---

4. General advice

• Work on the spacing - try to put spaces between operands and operators as in if(length % 2 == 0), in places like for loops : for(int i = 0; i < length; i++) and in function parameters : scanf("%Iu", &count);
• Put comments before the thing they're describing, not at the end of the line - this way if you want to make it a multi-line comment you won't have problems and it's more readable that way e.g.

like this:

//Function for creating array of numbers.
int* MakeArray(size_t);
//Function for calculating the median.
int Median(int*, size_t);

This is just an example - I think the function names are descriptive enough not to put comments there, they will clutter the view :

int* MakeArray(size_t);
int Median(int*, size_t);

---

EDIT

What I didn't initially remember is that to calculate the median, you have to use a sorted array. You should probably do that separately

int *array = MakeArray(count);
qsort(array, count, sizeof(int), compare)
float result = Median(count, array);

The qsort is a function from stdlib.h. You have to also declare a function compare to use in it e.g.

int compare(const void* a, const void* b)
{
     int int_a = *((int*) a);
     int int_b = *((int*) b);

     if(int_a == int_b) return 0;
     else if (int_a < int_b) return -1;
     else return 1;
}

This is taken from https://stackoverflow.com/a/3893967/7931009