Suggestions for code improvement

0

Good evening, I created a Joga from the Forca in C ++ as an exercise and would like to receive suggestions for code improvement. Thank you in advance for being able to respond.

Follow the code:

#include <windows.h>
#include <iostream>
#include <string>

std::string palavfr_secreta;
std::string dica;
std::string esconde;

short chances;
short acertos;
short erros;

void jogo(){

    std::string letra_digitada;
    std::string letra_secreta;
    std::string letra_esconde;

    short contador=0;

    std::cout << "================== " << std::endl;
    std::cout << "Dica-------------> " << dica << std::endl;
    std::cout << "Palavra secreta--> " << esconde << std::endl;
    std::cout << "Chances----------> " << chances << std::endl;
    std::cout << "================== " << std::endl << std::endl;
    std::cout << "Digite uma letra-> ";
    std::cin >> letra_digitada;

    std::cin.ignore();

    if(letra_digitada.size()>1){

        system("cls");

        std::cout << "DIGITE SOMENTE UMA LETRA!" << std::endl;

        jogo();

    }else{

        for(unsigned i=0; i<palavfr_secreta.size(); i++){

            letra_secreta=palavfr_secreta.at(i);
            letra_esconde=esconde.at(i);

            if(letra_digitada.compare(letra_esconde)==0){

                system("cls");

                std::cout << "Você não pode digitar a mesma letra mais de uma vez" << std::endl;

                chances--;

                break;

            }else if(letra_digitada.compare(letra_secreta)==0){

                system("cls");

                esconde.at(i)=letra_digitada.at(0);

                contador++;
                acertos++;

            }else if(i==palavfr_secreta.size()-1 && contador==0){

                system("cls");

                std::cout << "Letra não correspondente" << std::endl;

                chances--;
                erros++;
            }
        }
    }

    if(chances==0){

        system("cls");

        std::cout << "======================================================================" << std::endl;
        std::cout << "Você perdeu! A frase ou palavra secreta era " << "'" << palavfr_secreta << "'" << std::endl;
        std::cout << "Total de acertos -> " << acertos << std::endl;
        std::cout << "Total de erros -> " << erros << std::endl;
        std::cout << "======================================================================" << std::endl;

    }else if(esconde.compare(palavfr_secreta)==0){

        system("cls");

        std::cout << "=======================================================================" << std::endl;
        std::cout << "Você acertou! A frase ou palavra secreta era " << "'" << palavfr_secreta << "'" << std::endl;
        std::cout << "Total de acertos -> " << acertos << std::endl;
        std::cout << "Total de erros -> " << erros << std::endl;
        std::cout << "=======================================================================" << std::endl;

    }else{

        jogo();
    }
}

int main(){

    std::cout << "===============Jogo da Forca===============" << std::endl;
    std::cout << "Digite uma palavra ou frase secreta-> ";
    getline(std::cin, palavfr_secreta);

    system("cls");

    std::cout << "Escreva uma dica-> ";
    getline(std::cin, dica);

    system("cls");

    std::cout << "Defina o número de Chances-> ";
    std::cin >> chances;

    for(unsigned i=0; i<palavfr_secreta.size(); i++){

        if(palavfr_secreta.at(i)==' '){
            esconde+=" ";
        }else{
            esconde+="-";
        }
    }

    system("cls");

    jogo();

    return 0;
}
    
asked by anonymous 24.01.2018 / 00:41

2 answers

1

Our format is not very code review, this applies more to the Code Review , yet here are some of my improvements you can do:

  • Avoid doing loops / loops / cycles at the expense of recursion. Something you are doing here:

    void jogo(){
        if(chances==0){
            ...
        }else if(esconde.compare(palavfr_secreta)==0){
            ...
        }else{
            jogo();
        }
    

    Not only is it much less efficient, but you can even crash the program with the classic Stack Overflow Exception . This is because it is constantly opening new functions on the stack until it potentially explodes.

    How to solve this problem is by transforming the recursion into a loop with while or for for example.

  • When accessing a letter of a string is more idiomatic access by the indexing operator, [] instead of palavfr_secreta.at(i)

  • Try to put variables as close to their use as possible:

    void jogo(){
    
        std::string letra_digitada;
        std::string letra_secreta;
        std::string letra_esconde; //<----------- declarada aqui
    
        short contador=0;
    
        std::cout << "================== " << std::endl;
        std::cout << "Dica-------------> " << dica << std::endl;
        std::cout << "Palavra secreta--> " << esconde << std::endl;
        std::cout << "Chances----------> " << chances << std::endl;
        std::cout << "================== " << std::endl << std::endl;
        std::cout << "Digite uma letra-> ";
        std::cin >> letra_digitada;
    
        std::cin.ignore();
    
        if(letra_digitada.size()>1){
    
            system("cls");
    
            std::cout << "DIGITE SOMENTE UMA LETRA!" << std::endl;
    
            jogo();
    
        }else{
    
            for(unsigned i=0; i<palavfr_secreta.size(); i++){
    
                letra_secreta=palavfr_secreta.at(i);
                letra_esconde=esconde.at(i); //<---------utilizada aqui
    

    Although it may be (potentially) more efficient in some cases, it will considerably impair reading because it is difficult to know where the variable is being used. And if it's an optimization perspective, it's probably premature optimization that even the compiler will be able to do.

  • To find out if a given caratere exists in a string has the method find of string which makes it easier for you to convert your for and first if :

    for(unsigned i=0; i<palavfr_secreta.size(); i++){
        letra_secreta=palavfr_secreta.at(i);
        letra_esconde=esconde.at(i);
    
        if(letra_digitada.compare(letra_esconde)==0){
            ....
    

    In

    if (esconde.find(letra_digitada) != std::string::npos){
    

    Assuming that letra_digitada is char .

  • Try using the most appropriate types for your values. When reading char as in letra_digitada , read to a variable of type char . This makes it easier for you to compare.

    Being able to transform this:

    }else if(letra_digitada.compare(letra_secreta)==0){
    

    In this

    }else if(letra_digitada == palavra_secreta[i]){
    
  • The block you have for when you can not find the letter you typed in for :

    for(unsigned i=0; i<palavfr_secreta.size(); i++){
        ...
        }else if(i==palavfr_secreta.size()-1 && contador==0){
            system("cls");
            std::cout << "Letra não correspondente" << std::endl;
            chances--;
            erros++;
        }
    }
    

    It should actually be out of for . You would do this by putting within%% of% only the% of% of when the letter exists and changing the value of a flag . Then following the for tests the flag to find out that it is correct in some letter.

Applying all these improvements I suggested the code would look like this:

#include <windows.h>
#include <iostream>
#include <string>

std::string palavra_secreta,dica,esconde;    
short chances,acertos,erros;

void jogo(){
    //nova condição de fim do jogo
    while (acertos < palavra_secreta.size() && chances > 0){ 
        system("cls"); //apenas limpa aqui e não dentro do for como tinha
        std::cout << "================== " << std::endl;
        std::cout << "Dica-------------> " << dica << std::endl;
        std::cout << "Palavra secreta--> " << esconde << std::endl;
        std::cout << "Chances----------> " << chances << std::endl;
        std::cout << "================== " << std::endl << std::endl;
        std::cout << "Digite uma letra-> ";

        char letra_digitada; //agora como char
        std::cin >> letra_digitada;

        if (esconde.find(letra_digitada) != std::string::npos){//testa se existe com find
            system("cls");
            std::cout << "Você não pode digitar a mesma letra mais de uma vez" << std::endl;
            chances--;
        }
        else {
            int novos_acertos = 0;

            for(unsigned int i=0; i < palavra_secreta.size(); i++){
                if(letra_digitada == palavra_secreta[i]){ //apenas atualiza as letras
                    esconde[i]=letra_digitada;
                    novos_acertos++;
                    acertos++;
                }
            }

            if (novos_acertos == 0){ //se não acertou nenhuma mostra mensagem
                system("cls");
                std::cout << "Letra não correspondente" << std::endl;
                chances--;
                erros++;
            }
        }

        if(chances == 0){
            system("cls");
            std::cout << "======================================================================" << std::endl;
            std::cout << "Você perdeu! A frase ou palavra secreta era " << "'" << palavra_secreta << "'" << std::endl;
            std::cout << "Total de acertos -> " << acertos << std::endl;
            std::cout << "Total de erros -> " << erros << std::endl;
            std::cout << "======================================================================" << std::endl;
        }else if(esconde.compare(palavra_secreta) == 0){
            system("cls");
            std::cout << "=======================================================================" << std::endl;
            std::cout << "Você acertou! A frase ou palavra secreta era " << "'" << palavra_secreta << "'" << std::endl;
            std::cout << "Total de acertos -> " << acertos << std::endl;
            std::cout << "Total de erros -> " << erros << std::endl;
            std::cout << "=======================================================================" << std::endl;
        }
    }
}

int main(){
    std::cout << "===============Jogo da Forca===============" << std::endl;
    std::cout << "Digite uma palavra ou frase secreta-> ";
    getline(std::cin, palavra_secreta);

    system("cls");
    std::cout << "Escreva uma dica-> ";
    getline(std::cin, dica);

    system("cls");
    std::cout << "Defina o número de Chances-> ";
    std::cin >> chances;

    for(unsigned i=0; i<palavra_secreta.size(); i++){
        esconde += palavra_secreta[i]==' ' ? " ":"_"; //com ternário para ficar simples
    }

    jogo();
    return 0;
}
    
24.01.2018 / 02:04
0

I realized that for a person to play another has to enter the word and give the tip, Then you could save several words per category in a file and randomly grab them. example in your menu would have something like choose a theme.
-animals
-fruits
-cores etc ...
then the player would choose animals and his program would draw a word from the animals file. if you want to make look the functions: rand () and srand () for randomness.
and the library: fstream to open files.

    
24.01.2018 / 01:07