I'm mainly just looking for syntax errors.
Syntax errors are only the beginning. C will not check anything for you. You have to check if files opened, if input scans worked, if values are inside array bounds. If you don't, that's how you get segfaults.
For files, the usual pattern is to try to open the file, check its return value, and then handle the error. fopen
returns NULL
on error and sets errno
. errno
is a global which holds what sort of error happened like "file not found", but it's a number. strerror
is handy to turn it into an error message. Finally fprintf
is like printf
but can print to things other than stdout
. In this case stderr
. Both stdout
and stderr
normally appear on the screen, but they can be separated.
FILE *fp = fopen(path, mode);
if( fp == NULL ) {
// This will print something like "Could not open testcase_small_sorted.txt: No such file or directory"
fprintf(stderr, "Could not open file %s: %s", path, strerror(errno));
exit(1);
}
But now we need to copy this five times because the code repeats fopen
. Instead of each case opening the file, what if it just picked a filename? And since each filename has a number, what if they were just in an array?
const char *files[] = {
NULL, // 0 is not used
"testcase_small_sorted.txt",
"testcase_large_sorted.txt",
"testcase_small_nomatch_sorted.txt",
"hidden_small_sorted.txt",
"hidden_large_sorted.txt"
};
const char *path = files[whichfile];
What if they input something that's not a number? Or what if the number is out of range? These need to be checked for as well. scanf
will return the number of items matched. We expect 1. If we get anything else it didn't work.
// Check that we read an integer.
if( scanf("%d", &whichfile) != 1 ) {
fprintf(stderr, "Please enter 1-5.\n");
exit(1);
}
// Check that it's in range.
if( (whichfile < 1) || (5 < whichfile) ) {
fprintf(stderr, "Please enter 1-5.\n");
exit(1);
}
Note that scanf
has a lot of problems and should be swiftly discarded once you learn things like fgets
and sscanf
.
int lengths[1000000]
is 4 to 8 megabytes (1 million integers at 4 or likely 8 bytes per integer) and might get you an eponymous stack overflow. Your algorithm is O(n^2) which means if there really were 1,000,000 items it would take 1,000,000,000,000 iterations to find all the pairings and your class will probably be done before the program is.
(A little hint to improve the algorithm: if the numbers are sorted, and you're looking for two of them to sum X, do you need to check every number?)
I think you're meant trust n
from the file for how many to read. Normally you don't have this, it's a crutch to let you read input without using dynamic memory (you'll be taught that later). Even if you had it, you wouldn't trust it anyway in production code; if it lies you'll walk out of your array bounds. But for this exercise that's fine.
// Read the first two lines, the sum and the number of remaining lines.
// Note that `fscanf` can also fail and needs to be error checked.
// I'll leave that as an exercise.
fscanf(fp, "%d", &x);
fscanf(fp, "%d", &n);
// Use `n` to allocate just enough space on the stack.
int lengths[n];
// Read the rest.
// Use braces, even for one line.
// They avoid a very silly and hard to debug mistake.
for (i=0;i<n;i++) {
fscanf(fp, "%d", &lengths[i]);
}
fclose(fp);
There's no need to put x and n into lengths
. They aren't meant to be summed and you skip over them anyway by starting your loops at 2.
With that fixed the loops can start at 0. There is a mistake in the inner loop, it should check j < n
not i < n
.
for(i = 0; i < n; i++)
{
for(j = 0; j < n; j++) // <<--- j < n, not i < n.
{
if(lengths[i] + lengths[j] == x)
{
printf("Found: %d + %d == %d\n", lengths[i], lengths[j], x);
}
}
}