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 int
s - 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