4
\$\begingroup\$

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;
 }
asked Oct 9, 2020 at 13:57
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

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.

answered Oct 9, 2020 at 14:36
\$\endgroup\$
9
  • \$\begingroup\$ Thanks a lot understood :) Also this is probably the best welcome I have ever received on any SE so many upvotes \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Oct 9, 2020 at 15:49
  • \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ Commented 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() and printf(), dwarfing the concern about using a separate average(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\$ Commented Oct 10, 2020 at 16:09
1
\$\begingroup\$

... 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. Use double 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"
answered Oct 10, 2020 at 17:07
\$\endgroup\$
13
  • \$\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\$ Commented 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 different average(size_t length, const int array[]) versus different main(). Profile the whole, not the pieces. \$\endgroup\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Oct 10, 2020 at 17:43

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.