0

I tried making a game(beginner project), similar to atlas, wherein you start by quoting a country and the next guy has to say another country with the last letter of your country and so on. I wanted to see if this is the best version or whether it could be made better by removing some of the repetitive code and implementing better concepts.

This is the code

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

int is_present(char *country,char *countries[]){
    int c=0;
    for(int i =0;i<195;i++){
        if(strcmp(country,countries[i])==0){
            c=1;
            countries[i]="\0";
            break;
        }
    }
    return c;
}

int main(void)
{
    char *countries[] = {"afghanistan", "albania", "algeria", "andorra", "angola", "antigua and deps", "argentina", "armenia", "australia", "austria", "azerbaijan", "bahamas", "bahrain", "bangladesh", "barbados", "belarus", "belgium", "belize", "benin", "bhutan", "bolivia", "bosnia herzegovina", "botswana", "brazil", "brunei", "bulgaria", "burkina", "burundi", "cambodia", "cameroon", "canada", "cape verde", "central african republic", "chad", "chile", "china", "colombia", "comoros", "congo", "costa rica", "croatia", "cuba", "cyprus", "czech republic", "denmark", "djibouti", "dominica", "dominican republic", "east timor", "ecuador", "egypt", "el salvador", "equatorial guinea", "eritrea", "estonia", "ethiopia", "fiji", "finland", "france", "gabon", "gambia", "georgia", "germany", "ghana", "greece", "grenada", "guatemala", "guinea", "guinea-bissau", "guyana", "haiti", "honduras", "hungary", "iceland", "india", "indonesia", "iran", "iraq", "ireland", "israel", "italy", "ivory coast", "jamaica", "japan", "jordan", "kazakhstan", "kenya", "kiribati", "north korea", "south korea", "kosovo", "kuwait", "kyrgyzstan", "laos", "latvia", "lebanon", "lesotho", "liberia", "libya", "liechtenstein", "lithuania", "luxembourg", "macedonia", "madagascar", "malawi", "malaysia", "maldives", "mali", "malta", "marshall islands", "mauritania", "mauritius", "mexico", "micronesia", "moldova", "monaco", "mongolia", "montenegro", "morocco", "mozambique", "myanmar", "namibia", "nauru", "nepal", "netherlands", "new zealand", "nicaragua", "niger", "nigeria", "norway", "oman", "pakistan", "palau", "panama", "papua new guinea", "paraguay", "peru", "philippines", "poland", "portugal", "qatar", "romania", "russia", "rwanda", "saint kitts and nevis", "saint lucia", "saint vincent and the grenadines", "samoa", "san marino", "sao tome and principe", "saudi arabia", "senegal", "serbia", "seychelles", "sierra leone", "singapore", "slovakia", "slovenia", "solomon islands", "somalia", "south africa", "south sudan", "spain", "sri lanka", "sudan", "suriname", "swaziland", "sweden", "switzerland", "syria", "taiwan", "tajikistan", "tanzania", "thailand", "togo", "tonga", "trinidad and tobago", "tunisia", "turkey", "turkmenistan", "tuvalu", "uganda", "ukraine", "united arab emirates", "united kingdom", "united states of america", "uruguay", "uzbekistan", "vanuatu", "vatican city", "venezuela", "vietnam", "yemen", "zambia", "zimbabwe"};
    int d=1;
    int num_of_players;
    printf("Enter number of players: ");
    scanf("%d", &num_of_players);
    fflush(stdin);
    char *word1=(char *)malloc(1024*sizeof(char));
    printf("Enter word player %d: ",d);
    scanf("%[^\n]s",word1);
    word1=realloc(word1,(strlen(word1)+1)*sizeof(char));
    fflush(stdin);
    if(is_present(word1,countries)==1){ //checks whether word1 is present in countries
        d+=1;
        char *word2=(char *)malloc(1024*sizeof(char));
        printf("Enter country player %d: ",d);
        scanf("%[^\n]s",word2);
        fflush(stdin);
        word2=realloc(word2,(strlen(word2)+1)*sizeof(char));
        int c=0;
        do{
            if(is_present(word2,countries)==1){ //checks if word2 is present in countries
                if(word1[strlen(word1)-1]==word2[0]){
                    printf("Well Done Player %d\n",d);
                    d+=1;
                    if(d==(num_of_players)+1){
                        d=1;
                    }
                    char *s=word1;
                    char *temp=(char *)malloc(1024*sizeof(char));
                    printf("Enter country player %d: ",d);
                    scanf("%[^\n]s",temp);
                    fflush(stdin);
                    temp=realloc(temp,(strlen(temp)+1)*sizeof(char));
                    word1=word2;
                    word2=temp;
                    temp=s;
                    free(temp);
                    word1=realloc(word1,(strlen(word1)+1)*sizeof(char));
                    word2=realloc(word2,(strlen(word2)+1)*sizeof(char));
                    c=1;
                }
                else if(word1[strlen(word1)-1]!=word2[0]){
                    printf("Last letter of %s is not equal to the first letter of %s\n",word1,word2);
                    printf("Player %d Loses",d);
                    c=0;
                    free(word1);
                    free(word2);
                }   
            }
            else{ //word2 not valid
                printf("The country %s is either not present or used before.\n",word2);
                printf("Player %d Loses",d);
                c=0;
                free(word1);
                free(word2);
            }
        }while(c>0);
    }
    else if(is_present(word1,countries)==0){ //word1 not a valid country
        printf("The country %s is not a valid country\n",word1);
        printf("Player %d Loses",d);
        free(word1);
    }
    return 0;
}

The output also looks more like a program running than a game so any suggestions to make it look like a user interface are appreciated.

Enter number of players: 5      
Enter word player 1: albania
Enter country player 2: afghanistan
Well Done Player 2
Enter country player 3: nigeria
Well Done Player 3
Enter country player 4: azerbaijan
Well Done Player 4
Enter country player 5: niger
Well Done Player 5
Enter country player 1: romania
Well Done Player 1
Enter country player 2: barbados
Last letter of romania is not equal to the first letter of barbados
Player 2 Loses

Edit: Also, the game stops after one player loses. I want to make it so that it continues after the player loses but they no more get their turn. In the end, the game only stops when there is just one player remaining and he is declared the winner. Any ideas, suggestions, or code would be helpful.

Thanks in advance

vedbanerjee
  • 3
  • 2
  • First issue I see, way too many unnecessary calls to malloc and realloc. Do you really think any word is going to be 1024 characters? You can allocate a 64 byte buffer on the stack in your main function and utilise that instead. – Irelia Jun 27 '22 at 01:34
  • 1
    Second issue: Your list of countries probably shouldn't be on the stack and better off as a global variable. – Irelia Jun 27 '22 at 01:35
  • fflush on stdin is not standard and UB. It's meant for the output stream. Instead of scanf, consider fgets to completely read your input stream. – Irelia Jun 27 '22 at 01:41
  • Your `is_present` could just return 1 if strcmp == 0, and return 0 otherwise, rather than utilising an extra variable and break. – Irelia Jun 27 '22 at 02:03
  • 1
    You could replace `is_present` by a call to the library function `bsearch`, (to do a binary search, since your countries are in order.) – Neil Jun 27 '22 at 05:50

1 Answers1

2

Here are some suggestions to simplify your code :

  • The realloc is unnecessary (scanf adds a '\0' if that is your reasoning to add 1 char).

  • Just reuse the same buffers for player input instead of freeing and allocating every time.

  • You can also just use a single buffer for all player input and keep the last letter of the previous answer in a char. This would make the swapping of buffers unnecessary, but without the previous answer you would have to change one of the losing messages to something like "The first letter of %s is not %c\n".

  • Treating the first play separately leads to code duplication, I would just add a condition before testing the country's first character.

  • The else if condition is unnecessary, just use else.

  • The format string should be "%[^\n]", the 's' is unnecessary (it makes the scanf try to scan an 's')

  • Also, beware that if the scanf fails (the player just presses enter), the buffer will contain garbage.

  • fflush should be used with output streams only, fflush(stdin) is undefined behavior (apparently it works on windows but I can't test), use getchar() or getc(stdin) instead to delete the '\n'.

  • As another comment said, you can use fgets instead of scanf, it will empty the input stream but ... store the '\n' in your buffer (see Removing trailing newline character from fgets() input).

  • <ctype.h> is not used.

  • Missing '\n' after "Player %d Loses", also those printf should be moved at the end to avoid code duplication.

  • You can use the modulo operator to cycle through the players: d = (d % num_of_players) + 1. Also, the first instance of d += 1 doesn't check the number of players, which is a bug when num_of_players = 1.

A few remarks, unrelated to code bloat:

  • Use meaningful variable names, instead of c or d.

  • You could return a bool with the is_present function (include <stdbool.h>).

  • The countries array is defined in main, but its size appears as a "magic number" in the is_present function, this is bad practice. The array and the size should probably be globals, or static members of the is_present function.

The user interface part is a whole other beast, you'd need to learn a GUI library such as Qt or Tcl/Tk. If you keep a console application and just want to make it a bit fancier, look up ANSI escape codes for moving the cursor, clearing the terminal, using colors, etc. (List of ANSI color escape sequences)

Hope this helps

Sref
  • 36
  • 3