I have started with C and I made this code and as the title suggests this program can calculate the average score of a given number of scores. Now my major concern is, is it more efficient to get the number of scores as a Command Line Argument or is the current approach better incase if sometime in the future someone designs a Interactive Front-End for it?
Also I am using the cs50 library I am not sure if it is allowed here please tell me if it's not so I'll keep that in mind in the future.
#include <cs50.h>
#include <stdio.h>
float average(int length, int array[]);
int main(void)
{
int i, j;
printf("This Program will Help you Compute the Average Score \n"); //Welcome Message
int Total = get_int("How Many Scores do you wish to Use: "); //Asks User for How Many Scores
int scores[Total]; //Stores Scores in an Array
for (i=0; i < Total; i++) //A Loop to keep Asking for Scores
{
scores[i]=get_int("Enter Score %d \n", i+1); //i+1 is used as it would be better to start asking from Score 1 so that it avoids confusion
}
printf("The Average score is %f", average(Total, scores));
}
float average(int length, int array[])
{ int sum = 0;
for (int j=0; j < length; j++)
{
sum += array[j];
}
return sum/(float)length;
}
2 Answers 2
Reduce the number of loops
The most time-consuming part of your program is to loop through the array. Notice that you have done this twice, why can't you keep a track of the total sum while taking the input?
#include <stdio.h>
float average(int sum,int number);
int main(void)
{
int number_of_integers,number,sum = 0;
printf("Enter the number of integers: ");
scanf("%d",&number_of_integers);
for(int i = 0;i < number_of_integers;i++)
{
printf("Enter: ");
scanf("%d",&number);
sum+=number;
}
printf("Average = %d",average(sum,number_of_integers));
}
float average(int sum, int number)
{
return sum/(float)number;
}
This reduces the number of loops from 2
into 1
. And completely removes the need of having an array of integers. Making your program both memory efficient and fast.
-
\$\begingroup\$ Thanks a lot understood :) Also this is probably the best welcome I have ever received on any SE so many upvotes \$\endgroup\$FoundABetterName– FoundABetterName2020年10月09日 15:31:57 +00:00Commented Oct 9, 2020 at 15:31
-
\$\begingroup\$ i just checked you github profile and man you learnt coding at such a young age can you refer to some resources which you used? I am totally new to coding \$\endgroup\$FoundABetterName– FoundABetterName2020年10月09日 15:39:07 +00:00Commented Oct 9, 2020 at 15:39
-
1\$\begingroup\$ Thanks a lot as a side question how long did it take you to master C++? I am studying C as I was following the cs50 course and they started with this \$\endgroup\$FoundABetterName– FoundABetterName2020年10月09日 15:49:30 +00:00Commented Oct 9, 2020 at 15:49
-
\$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$FoundABetterName– FoundABetterName2020年10月09日 15:51:46 +00:00Commented Oct 9, 2020 at 15:51
-
\$\begingroup\$ "The most time-consuming part of your program is to loop through the array." --> The most time consuming is the execution of complex functions like
scanf()
andprintf()
, dwarfing the concern about using a separateaverage(int length, int array[])
. Better to focus on reducing O(n) ( which was done here with memory usage) than concerns about "number of loops from 2 into 1"`. \$\endgroup\$chux– chux2020年10月10日 16:09:37 +00:00Commented Oct 10, 2020 at 16:09
... major concern is, is it more efficient to get the number of scores as a Command Line Argument or is the current approach better in case if sometime in the future someone designs a Interactive Front-End for it?
"more efficient" needs to focus on order of complexity , else one wastes valuable coding effort in premature optimizations. OP's code run-time is O(n). It does not get better in run-time using using 1 loop or 2.
current approach better incase if sometime in the future someone designs a Interactive Front-End for it?
Current is better.
Code architecture designed for growth is a good idea. A good strategy is to separate input, processing and output.
In OP's small code, loops can be combined (saves memory), yet for growth and real larger code, and portability, maintain separations.
// Pseudo code
main()
setup()
input()
process_data()
output()
cleanup()
Set aside small linear time improvement concerns and look to proper full range functionality
- Below code can readily overflow
sum += array[j]
. float
in C is rarely the preferred type. Usedouble
and reduce precision loss.- Array sizing best done with
size_t
. - Consider future trends with function signatures (which OP followed with
length
1st). - Use
const
when referenced data is not changed. Allows wider usage and conveys intent. - Consider alternatives to casting.
- Trust your compiler to make at least basic optimizations. Code for clarity.
OP's
float average(int length, int array[]) { // length more than INT_MAX?
int sum = 0;
for (int j=0; j < length; j++) {
sum += array[j]; // Overflow ?
}
return sum/(float)length; // low precision FP
}
Alternative
double average(size_t length, const int array[]) {
long long sum = 0; // or double sum = 0.0 is very large input expected.
for (size_t j=0; j < length; j++) {
sum += array[j];
}
double avg = sum;
return avg/length;
}
Minor: Output improvements
"%f"
is weak. Consider "%g"
. It is more informative when the value is smaller and not so noisy when the value is large.
No need for spaces before "\n"
. Such spaces are surprising to find in output. Avoids surprising effects.
// Enter Score %d \n"
Enter Score %d\n"
-
\$\begingroup\$ I performed your
get_average()
1000000 times, with the size of the array as 1000. The input of the method being the same, your code slowed down a lot whereas my solution had the sum pre-calculated so it was instantaneous. I am really confused because I'm sure you have way more knowledge than me, can you explain? \$\endgroup\$user228914– user2289142020年10月10日 17:29:53 +00:00Commented Oct 10, 2020 at 17:29 -
\$\begingroup\$ @AryanParekh
average()
is only called once in OP's code. Why call 1000000 times? Looks like you are trying to assess differentaverage(size_t length, const int array[])
versus differentmain()
. Profile the whole, not the pieces. \$\endgroup\$chux– chux2020年10月10日 17:35:15 +00:00Commented Oct 10, 2020 at 17:35 -
\$\begingroup\$ I really think that is a bad excuse, But, What about memory? It is extremely in-efficient if you look into it form that side \$\endgroup\$user228914– user2289142020年10月10日 17:38:03 +00:00Commented Oct 10, 2020 at 17:38
-
\$\begingroup\$ You are hanging on the exception that the
get_average()
function will be called only once as the question does the same, yet you talk about long term code re-use \$\endgroup\$user228914– user2289142020年10月10日 17:41:44 +00:00Commented Oct 10, 2020 at 17:41 -
\$\begingroup\$ @AryanParekh "I really think that is a bad excuse" --> Perhaps your insight into programming exceeds mine . Re: memory in-efficiency. Answers already covers that with "saves memory". Coding to optimize trivial programs prevents larger concerns about code re-use. \$\endgroup\$chux– chux2020年10月10日 17:43:13 +00:00Commented Oct 10, 2020 at 17:43