# C arrays and loops optimization

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 can not be the same number used twice). I feel I have come to an okay solution but there are a few issues. I want to try and separate the code into separate functions and currently if the text file inputted ends with a newline char the program loops once too far and hence prints out the calculation for a non existent line, I'm not sure how to get around this.

#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);
}


So what I'm looking for is code optimization advice and help

Thanks

-

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
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.

• 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.

-
This has been really great help in cleaning up and organising my code. It's a lot more legible now. Thanks! –  BradStevenson Nov 10 '12 at 16:01