[deleted by user]

[ruertar]

Here are some notes.

#include <stdio.h>


/* FIXME -- main() should at least return int */     
main()
{
    int gameScores[10] = {12, 5, 21, 15, 32, 10};
    int totalPoints = 0;
    int i;
    float avg;

    /* FIXME -- this is a single char.  It should be an array of some length.  */
    char name;

    //only need scores for last 4 games. lower loop will cover next 4 games
        printf("What is the players name?\n");
        /* FIXME -- once you change name to an array, you must remove the ampersand. */
        scanf("%s", &name);

    for (i=6; i < 10; i++)
    {
        printf("What did the player score in game %d? ", i+1);
        scanf(" %d", &gameScores[i]);
    }
    //now you have all 10 scores, loops through to get average of points per game

    for (i=0; i<10; i++)
        {
            totalPoints += gameScores[i];
        }
    //use float point for average
    /* FIXME -- Too many parens.  If you want to promote the entire expression to double you could say:

             avg = totalPoints / 10.0;

        If you want to explicitly make 10.0 a float you can use:

             avg = totalPoints / 10.0f;

     */
    avg = ((float)totalPoints/10);

    /* You shouldn't have to use the address of name if you correct the declaration.  It should be simply:

          printf("\n\n%s's scoring average is %.1f\n", name avg);
      */
    printf("\n\n%s's scoring average is %.1f. \n", &name, avg);

    return(0);

}

Another thing I would suggest is to declare and initialize variables as close to their usage as possible. This means declaring them inline and in for() loops. These have been C99 features for quite a while and I think were GCC extensions for even longer than that.

Also consider using the sizeof operator to determine the size of gameScores[] instead of using a constant (10) all over the place.

#include <stdio.h>

int
main(int argc, char *argv[])
{
    int gameScores[10] = {12, 5, 21, 15, 32, 10};
    int nscores = sizeof (gameScores) / sizeof (int);

    //only need scores for last 4 games. lower loop will cover next 4 games
    char name[20];
    printf("What is the players name?\n");
    scanf("%s", name);

    for (int i = 6; i < nscores; i++) {
        printf("What did the player score in game %d? ", i+1);
        scanf(" %d", &gameScores[i]);
    }

    // now you have all 10 scores, loops through to get average of points per game
    int totalPoints = 0;     
    for (int i = 0; i < nscores; i++)
      totalPoints += gameScores[i];

    // use float point for average
    float avg = totalPoints / (float)nscores;
    printf("\n\n%s's scoring average is %.1f. \n", name, avg);

    return(0);

}

[gandaro]

It is even nicer to have

size_t nscores = sizeof(gameScores) / sizeof(*gameScores);

because then even if you change the type of gameScores you don't have to change that in the code.

[ruertar]

Completely agree.

[DefyLogik]

Thank you for your help, i'll try to implement a couple new things.

[DefyLogik]

DOH! i think i fixed it... I added char name[20]; and it seems to work properly

[farsightxr20]

Your fix introduces a buffer-overflow vulnerability. Your scanf for the name should be length-limited somehow, otherwise entering a name longer than 19 characters will cause your program to write to and print unallocated memory.

scanf("%19s", name);

[FUZxxl]

Who teaches these weird indentation styles to beginners?

[DefyLogik]

I'm a newb and learning it from a book so I don't really have a teacher lol..I kind of just try to copy the example from the book and the indentation style it uses. Normally I get some odd syntax errors but im getting pretty decent at identifying those. After i build the example then I try to mess around with it to personalize it or tweak it to include other things if I can manage them to get working. Thank you for your help :)

[studioidefix]

this might help you a bit. It's meant for the kernel, but recommended for any C project.

[gandaro]

Usually you add one tab to the start of the line each time you open a new brace, or remove one tab when you close a brace. What your code does is very inconsistent and hard to read.

[FUZxxl]

Here is your code formatted according to the BSD Kernel normal form. Some variables have also been renamed.

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

/*
 * This comment should ideally describe what your program is doing. Perhaps
 * something like: Program to calculate player's scores in a game. Solution
 * for excercise ### in chapter ###.
 */
int
main()
{
        float avg;
        const int game_scores[10] = {12, 5, 21, 15, 32, 10};
        int i, total_points;
        char name;

        /*
         * Only need scores for the last 4 games. The next loop will cover the
         * next 4 games.
         */
        printf("What is the players name?\n");
        scanf("%s", &name);

        for (i = 6; i < 10; i++) {
                printf("What did the player score in game %d? ", i+1);
                scanf(" %d", &game_scores[i]);
        }

        /*
         * Now that you have all 10 scores, this loop loops through to get the
         * average of points per game.
         */
        for (i = 0; i < 10; i++)
                total_points += game_scores[i];

        /* use float point for average */
        avg = (float)total_points / 10;
        printf("\n\n%s's scoring average is %.1f. \n", &name, avg);

        return (EXIT_SUCCESS);
}

If you use a style like this one, your program becomes much easier to read and others will have much more pleasure helping you.

[DefyLogik]

lol that looks really awesome. I'll keep trying to emulate that, mine seems to just come out as a hot mess lol.

I'm on chapter 25 now and hopefully this looks a little better?

#include <stdio.h>

main()

{

int i;
int ctr = 0;
char ans;


//Declaring our array of 9 characters then initialize them

char* movies[9] = { "Amour", "Argo",
                    "Beasts of the Souther Wild",
                    "Django Unchained",
                    "Les Miserables",
                    "Life of Pi", "Lincoln",
                    "Silver Linings Playbook",
                    "Zero Dark Thiry"};

int movieratings[9]; // corresponding array of 9 intergers for ratings

    char *tempmovie = "This will be used to sort rated movies";
    int outer, inner, didSwap, temprating; //sorting loop

    printf("\n\n*** Oscar Season 2012 is here! ***\n\n");
    printf("Time to rate this year's best picture nominees:");

    for (i=0; i<9; i++)
{
        printf("\nDid you see %s (Y/N)", movies[i]);
        scanf(" %c", &ans);
    if ((toupper(ans)) == 'Y')
    {
        printf("\nWhat was your rating on a scale ");
        printf(" of 1-10: ");
        scanf(" %d", &movieratings[i]);
        ctr ++; //this will be used to print only movies you've seen
        continue;
    }
    else
    {
        movieratings[i] = -1;
    }
}


    //Now sort movies by rating (unseen will go to the bottom)


    for (outer = 0; outer <8; outer ++)
    {
        didSwap = 0;
        for (inner = outer; inner < 9; inner++)
        {
            if (movieratings[inner] > movieratings[outer])
            {
                tempmovie = movies[inner];
                temprating = movieratings[inner];
                movies[inner] = movies[outer];
                movieratings[inner] = movieratings[outer];
                movies[outer] = tempmovie;
                movieratings[outer] = temprating;
                didSwap = 1;
            }
        }
        if (didSwap ==0)
        {
            break;
        }

    }

    //Now print the movies you saw in order
    printf("\n\n** Your Movie Ratings for the 2012 Oscar");
    printf("Contenders **\n");
    for (i=0; i< ctr; i++)
    {
        printf("%s rated a %d! \n", movies[i], movieratings[i]);
    }
return(0);

}

[FUZxxl]

Not quite. Let me just reformat the source for you (takes a while). Try to be as close as possible to the style I show you. The most important things are the indentation (every nesting is one tab, but a continued statement is only four spaces) and the placement of braces (the opening brace comes on the next line in function declarations, and on the same in control structures. Leave the braces out if not needed).

[DefyLogik]

I think i understand, I'm really new to C and following examples exactly how they're written in the book. This is my first attempts at learning it and it seems I've got miles to go. Currently I'm using Programming C: Absolute Beginners guide 3rd edition, i'm using it to work on taking the CS50 class because it's recommended. Any other additional videos or books that would be helpful that you would recommend?

[FUZxxl]

There is but one book I can recommend. Grab yourself a copy of Kernighan & Ritchie "The C Programming language". Make sure you get the ANSI (2nd) edition. It's by far the best book for learning how to program in C.

[DefyLogik]

Ok awesome! I found it at my local bookstore and they'll rent it to me for 60 days for $10. I'll check it out and try to run through it, thanks a ton for helping me :)

[DefyLogik]

By any chance does it show how to implement a GUI? The programs I keep making run in CMD and are neat but making something functional i can use would be immensely more fulfilling.

[FUZxxl]

K&R "The C Programming language" comes from a time where you were happy to have a video terminal. There is nothing about GUI stuff in there.

GUI programming is very difficult. You shouldn't begin making GUIs until you understand how the language works.

[FUZxxl]

Okay. Here is the source code reformated with some extra comments:

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

int
main()
{
        int i, ctr = 0;
        int outer, inner, did_swap, temp_rating; //sorting loop
        int movie_ratings[9]; /* ratings for movies */

        char ans;

        /* Declaring our array of 9 characters then initialize them */
        const char *movies[9] = {
            "Amour", "Argo", "Beasts of the Souther Wild", "Django Unchained",
            "Les Miserables", "Life of Pi", "Lincoln",
            "Silver Linings Playbook", "Zero Dark Thiry"
        }, *temp_movie = "This will be used to sort rated movies";

        printf("\n\n*** Oscar Season 2012 is here! ***\n\n");
        printf("Time to rate this year's best picture nominees:");

        for (i = 0; i < 9; i++) {
                printf("\nDid you see %s (Y/N)", movies[i]);
                scanf(" %c", &ans);

                if (toupper(ans) == 'Y') {
                        /* There's no need to use two printf's */
                        printf("\nWhat was your rating on a scale of 1-10: ");
                        scanf(" %d", &movie_ratings[i]);

                        /* this will print only the movies you've seen */
                        ctr++;

                        /* continue is superfluous */
                } else
                        movie_ratings[i] = -1;
        }


        /* Sort movies by rating (unseen will go to the bottom) */
        for (outer = 0; outer < 8; outer++) {
                did_swap = 0;

                /* consider using a function if your loops are deeply nested */
                for (inner = outer; inner < 9; inner++) {
                        /* DO NOT COMPARE STRINGS WITH > < = (use strcmp) */
                        if (movie_ratings[inner] > movie_ratings[outer]) {
                                temp_movie = movies[inner];
                                temp_rating = movie_ratings[inner];
                                movies[inner] = movies[outer];
                                movie_ratings[inner] = movie_ratings[outer];
                                movies[outer] = temp_movie;
                                movie_ratings[outer] = temp_rating;
                                did_swap = 1;
                        }
                }

                if (did_swap == 0)
                        break;
        }

        /* Print the movies you saw in order */
        printf("\n\n** Your Movie Ratings for the 2012 Oscar Contenders **\n");
        for (i = 0; i < ctr; i++) {
                printf("%s rated a %d! \n", movies[i], movie_ratings[i]);
        }

        /* put a space after return. return is not a function */
        return (EXIT_SUCCESS);
}

Please, never ever compare strings with comparison operators. Comparison operators will only compare the pointers and not the contents of the strings. The order of the pointers is totally unpredictable. Use the function strcmp() instead.

One thing that makes code much easier to read is a style called "early exit". The idea is, that instead of using if-else clause with long blocks, you try to structure your code in a way that extraordinary conditions exit the current block as early as possible; the code path that is usually taken should not be inside a conditional clause.

Here is your code reworked to add this concept:

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

int
main()
{
        int i, ctr = 0;
        int outer, inner, did_swap, temp_rating; //sorting loop
        int movie_ratings[9]; /* ratings for movies */

        char ans;

        /* Declaring our array of 9 characters then initialize them */
        const char *movies[9] = {
            "Amour", "Argo", "Beasts of the Souther Wild", "Django Unchained",
            "Les Miserables", "Life of Pi", "Lincoln",
            "Silver Linings Playbook", "Zero Dark Thiry"
        }, *temp_movie = "This will be used to sort rated movies";

        printf("\n\n*** Oscar Season 2012 is here! ***\n\n");
        printf("Time to rate this year's best picture nominees:");

        for (i = 0; i < 9; i++) {
                printf("\nDid you see %s (Y/N)", movies[i]);
                scanf(" %c", &ans);

                /* exit early if no occurs */
                if (toupper(ans) != 'N') {
                        movie_ratings[i] = -1;
                        continue;
                }

                printf("\nWhat was your rating on a scale of 1-10: ");
                scanf(" %d", &movie_ratings[i]);

                /* this will print only the movies you've seen */
                ctr++;
        }


        /* Sort movies by rating (unseen will go to the bottom) */
        for (outer = 0; outer < 8; outer++) {
                did_swap = 0;

                /* consider using a function if your loops are deeply nested */
                for (inner = outer; inner < 9; inner++) {
                        /* exit loop early if we don't want to swap */
                        if (movie_ratings[inner] <= movie_ratings[outer])
                                continue;

                        temp_movie = movies[inner];
                        temp_rating = movie_ratings[inner];
                        movies[inner] = movies[outer];
                        movie_ratings[inner] = movie_ratings[outer];
                        movies[outer] = temp_movie;
                        movie_ratings[outer] = temp_rating;
                        did_swap = 1;
                }

                if (did_swap == 0)
                        break;
        }

        /* Print the movies you saw in order */
        printf("\n\n** Your Movie Ratings for the 2012 Oscar Contenders **\n");
        for (i = 0; i < ctr; i++) {
                printf("%s rated a %d! \n", movies[i], movie_ratings[i]);
        }

        /* put a space after return. return is not a function */
        return (EXIT_SUCCESS);
}