Reading integers from a file and counting the length of a descending subsequence

Question

I had a school project to solve, which I did. The basic focus of the project was to create the simplest code possible to solve it. This topic was created just to ensure if I wrote the code correctly.

The problem is the following:

We have declared a dynamic array which will take numeric values from a txt file. We want to read the array backwards and to make a simple check. Read every 'i' element in the array and check if the value of the array is greater than> the last one.If it is the value saved in the int called temp_max which will be the> value of the array in the i position. The program will continue to check every number and if a value in the array is greater than temp_max we will overwrite the value of the temp_max to the value of the array in that position.

#include <stdio.h>
#include <stdlib.h>
int main() {
 FILE *input, *output;
 int *children;
 int size, i;
 input = fopen("xxx.in", "r");
 if(input == NULL)
 return 1;
 fscanf(input,"%d",&size);
 if(size < 1 || size > 1000000)
 return 1;
 children = malloc(size * sizeof(int));
 if (children==NULL) 
 return 1; 
 for ( i = 0; i < size; i++ ) 
 {
 fscanf(input, "%d",&children[i]);
 }
 fclose(input);
 int total = 0;
 int temp_max = 0;
 for(i = size - 1; i >= 0; i--) 
 {
 if(children[i] > temp_max) {
 temp_max = children[i];
 total++;
 }
 }
 free(children);
 output = fopen("xxx.out", "w+");
 fprintf(output, "%d\n", total );
 fclose(output);
 //This is the end, my dear friend!
 exit(0);
}

• The int size was declared in one step above and is the size of the dynamic array.

• The int total calculate how many times a number is greater than another. Default value is zero cause, I believe that should be what is your opinion.

• The array hasn't any zero values or negative values at all.

The text file has the following format:

Example (xxx.in) 1:

5
9 6 5 4 2

Example (xxx.in) 2:

6
8959 5594 595 2 949 9

• chux: I see no need to read the entire file into an array. Just look for children[i] < temp_min in the forward direction.
• user61343: Good point. In my opinion though I though it would be safier to read all the numbers into an array and close the file than to have it open in all the process. Just my opinion. As for children[i] < temp_min I couldn't understand you. I want to check the numbers backwards and to find the max value. Why to mess thinks up. By the time that you didn't find any bug in my program I am happy! Thank you for your point! :)

Answer 1

int main() returns an int so instead of exit(0); write return 0; at the end of main, it is more natural. exit is more something you would use to panically exit a program.

When you write the file xxx.out, you don't check if opening the file succeeded, it always good to check return values of all runtime functions like you do when you read the other file.

Initialize all variables when you declare them to avoid any surprises. You have in some cases declared the variables where you use them, that is good. You should do that with all variable declarations instead of at top of main.

Check return value of fscanf() to see if it correctly got a number. I personally prefer fgets/sscanf when reading from file/stdin but that is me.

Maybe you could separate the functionality into some smaller functions e.g. readFile, writeFile and calculateTotal. It is a good habit to learn because programs have a tendency to grow with time, although for this particular case it may be overkill.

Another good habit is to write "why" comments, but I guess in this particular case it is quite self-evident what it does, although maybe a mention of the file format expected in the header of the program would be helpful for another programmer. :)

You could add some error messages to stderr when something is wrong instead of silently quitting, it makes troubleshooting easier e.g.

fprintf(stderr, "invalid size %d", size);

• user61343: well I understand your points. First you must know that our exersices will tested automatically on computers and not by examiners and we don't need to use error handling conditions. As for the functions I though about that but the program is small and I though it needn't. You think I should? The why commnets just have deleted for more eye-friendly look of my code. The fscanf work and tested. Now I understand from my school that every programer has his own way to make things. I want , if possible, to rewrite all of my code just to keep others with the excact point of view you have.
• user61343: As for the input file I make a check for NULL. As for my code is it working 100%?
• user61343: Do you think I need to declare all the ints to unsigned? I though not cause my program is small. What is your opinion?
• AndersK: @GeorgeGkas in this particular case it may be overkill, but in general it is better to split up functionality in functions. It makes the program easier to read. You can in one go see what the program does. readFile("xxx.in"); total = calculateTotal(...); writeFile("xxx.out"); Programs tend to grow with time e.g. adding error handling or other functionality.
• AndersK: @GeorgeGkas depends on your input data, you may wanna use long to be on the safe side. so far it looks like it will all fit snugly in an int.