Printing the largest sum from integers in a file

Question

I am writing a program for learning purposes that takes an input of a file structured similarly to:

13,22,13,14,31,22, 3, 1,12,10
11, 4,23, 7, 5, 1, 9,33,11,10
40,19,17,23, 2,43,35,21, 4,34
30,25,16,12,11, 9,87,45, 3, 1
1,2,3,4,5,6,7,8,9,10

Which will then read the values from the file, figure out and print the largest sum you can make from the digits on each line which is 50 or less (also cannot be the same number used twice).

I think I've come to a good solution, but there are a few issues. I want to try and separate the code into separate functions. I'm additionally looking for code optimization advice.

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

int main(int argc, char *argv[]) {
  FILE *fp;
  int hv, lv,line,i, linecount,val[5][10]; //hv + lv = the two highest values on a line, line=sum of hv and lv.
  int j=0;

  // test file succesfully opened
  if((fp=fopen(argv[1], "r")) == NULL){
    printf("Cannot open file.\n");
    exit(1);
  }

  //Store values in 2d array
  for(i=0;!feof(fp);i++){
    while(j<10){
      fscanf(fp, "%d,",&val[i][j++]);
    }
  j=0;
  }
  linecount=i-1; //set linecoutn to No of lines

//test and print result
 for(i=0; i<=linecount; i++){ // for each line
   hv=0, lv=0, line=0; //reset values for each line
   for(j=0;j<10;j++){ // for each value
     for(int a =0; a<10; a++) { //test against all in line
       if(a!=j && (val[i][j]+val[i][a]<=50)){ //if two values arent equal and sum is less than 50
     if((val[i][j]+val[i][a])>line){ //if value is greater than previous value
       hv=val[i][j];
       lv=val[i][a];
       line=hv+lv;
     }
       }
     }
   }
   printf("Line %d: largest pair is %d and %d, with a total of %d\n", i+1, hv, lv, line);
 }

 fclose(fp);
}

Answer 1

Brad, you code compiles cleanly, which is a good sign :-) Clearly it could be split into functions. An obvious approach would be to split the reading of the values from the processing of those values.

I would take a slightly different approach, but remember there is no one "correct" solution. I would read and process the file one line at time. So the pseudo code would be:

    for each line
        read the line data  (function)
        find greatest sum in line  (function)
        store sum if bigger than max value found

This approach avoids the need for a multi-dimensional array and means you can handle an arbitrary number of lines.

Note that nested loops are often not desirable, but are sometime necessary. Finding the greatest sum in the line looks like a case where a nested loop is optimal. But this double loop should be a function.

Also check the return value from fscanf. If you do this you can avoid processing a blank line.

A few comments on notation:

• Be consistent!!! This is important although it might seem trivial. Many people reading your code and seeing some places where a comma is followed by a space (preferred) and some where it is not will judge the code badly. Similarly, comments should be consistent - follow // by a space.

• Defining just one variable per line is usually preferred.

• Leave a space after keywords (if, else, for, while etc). Similarly, leave a space before and after = (or != etc). (these are my preferences, others might differ).

• Many people, including me, prefer code and comments to extend to no more than 80 columns.

• Send error messages to stderr and on error from system and library calls that set errno (the global error variable) use perror (which prints to stderr).

• Exit on failure with exit(EXIT_FAILURE);

• On completion, return a value from main, normally return EXIT_SUCCESS

• Oh and finally, delete the useless comments. Nearly all of your comments are worse than pointless - they add noise, not information.