5
\$\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;
  }
\$\endgroup\$
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.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot understood :) Also this is probably the best welcome I have ever received on any SE so many upvotes \$\endgroup\$ – FoundABetterName Oct 9 '20 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 Oct 9 '20 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 Oct 9 '20 at 15:49
  • \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ – FoundABetterName Oct 9 '20 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\$ – chux - Reinstate Monica Oct 10 '20 at 16:09
2
\$\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"
\$\endgroup\$
  • \$\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\$ – Aryan Parekh Oct 10 '20 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\$ – chux - Reinstate Monica Oct 10 '20 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\$ – Aryan Parekh Oct 10 '20 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\$ – Aryan Parekh Oct 10 '20 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 - Reinstate Monica Oct 10 '20 at 17:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.