Optimizing a program (c)

Question

i have my program:

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

int main (int argc, char *argv[])
{
    int r, line = 0, found = 0;
    float temp, t_tot = 0;
    char loc[32];


    FILE *fp;

    fp = fopen(argv[1], "r");

    if (fp == NULL)
    {
        printf ("Error opening the file\n\n'");
        exit(EXIT_FAILURE);
    }

    if (argc == 3)
    {
        while ((r = fscanf(fp, "%f %s\n", &temp, loc)) != EOF)
        {
            line++;

            if (r == 2)
            {
                if(strcmp(argv[2], loc) == 0)
                {
                    t_tot += temp;
                    found++;
                }
            }
            else
                printf ("Error, line %d in wrong format!\n\n", line);
        }

        printf ("The average temperature in %s is: %.1f\n\n", argv[2], (t_tot/found));
    }

    fclose(fp)

    return 0;

}

The program needs to read all the line and find the city i wrote on argv[2]. Than it will tell me the average temperature on that city, notifying me if a line in the file is in a wrong format.

I'm wondering how can i "optimize" this code to be more efficient and writing the same things in a more "compact" way. I'm a student so all suggestions are accepted.

This would be more suitable at http://codereview.stackexchange.com. — Oliver Charlesworth, Jun 23 '12 at 10:23
It is a common practice to check `argc` at the top of your `main`, and not return to the check in the loop. — Sergey Kalinichenko, Jun 23 '12 at 10:49
It is a common practice to use, for instance, `GNU getopt`, while we're at it :-) — Kos, Jun 23 '12 at 10:55

score 3 · Answer 1 · edited May 23 '17 at 12:04

Get a profiler like GNU GProf or perhaps AMD CodeAnalyst.

See also What's the best free C++ profiler for Windows?

Then compile your program with highest optimisation and try to examine which parts tend to take much time.

Optimising without a profiler is generally to be avoided.

While we're at it, your program doesn't really do any calculations that would take lots of time, and its performance is likely to be bound by the I/O (my guess).

Another thing you could do, instead of optimising, is to make it safe and correct- for instance so that it wouldn't crash if a string from the input file is longer than 32 characters.

score 1 · Answer 2 · answered Jun 23 '12 at 13:31

1

you can also optimize the object code with compiler optimization option. For gcc just add -O3 (or -O1 or -O2 , depending on the level of optimization) parameter.

answered Jun 23 '12 at 13:31

score 0 · Answer 3 · answered Jun 23 '12 at 10:24

0

What about changing

if (r == 2)
{
    if(strcmp(argv[2], loc) == 0)
    {
        t_tot += temp;
        found++;
    }
} else {
    printf ("Error, line %d in wrong format!\n\n", line);
}

into this, to avoid the nested if block:

if (r == 2 && strcmp(argv[2], loc) == 0) {
    t_tot += temp;
    found++;
} else if (r != 2) {
    printf ("Error, line %d in wrong format!\n\n", line);
}

Looks a lot cleaner to me!

answered Jun 23 '12 at 10:24

v1Axvw

3,054
3
26
40

Yeah, but I think it's worth it avoiding nested blocks. – v1Axvw Jun 23 '12 at 10:31
And why is this an improvement? Do these two even compile into different assembler? – Kos Jun 23 '12 at 10:57
@Kos it is not a time optimization, you are correct. But still I think it is makes the code look more organized. – v1Axvw Jun 23 '12 at 13:38

Optimizing a program (c)

3 Answers3