3
\$\begingroup\$

I'm a beginner programmer and i'm looking for interesting projects to improve my low skills. I tried one day to improve my skills by trying to create a small game in which a user owns a cat, and has to feed it and clean it. My main aim of the game(which yet has many illogical things like the never decreasing happiness and cleanness level) was to try and keep my code as clean and presentable as possible. The game is made in 4 different files, and I will paste them all below. I will be happy if anyone could give me help on understanding where should I improve.

Lets go into the code:

main.cpp

#include "Cat.h"
#include "pet game.h"
#include <iostream>
using namespace std;

int main() {
    //Create the user's profile
    //User();

    //Welcome messages
    cout
            << "Welcome to Unga bonga! Here you buy pets and raise them as your kids, and try to level up, in order to finish the game!"
            << endl;
    showHelp();

    //Command execution
    do {
        commandIdentify();
    } while (true);
}
Cat cat;
//User user;

void showHelp() {
    cout << "The commands in this game are:" << endl
            << "1. to feed your cat, type f" << endl
            << "2. to clean your cat, type c" << endl
            << "3. to check your cat's happiness, type h" << endl
            << "4. to check your cash balance, type m" << endl
            << "5. to check your level status, type l" << endl
            << "6. to display this help message, type ?" << endl
            << "7. to exit, type e" << endl
            << "8. Finally, to play this game, enjoy!" << endl;
}
    
void commandIdentify() {
    //take input
    cin >> command;
    //identify and execute
    switch (command) {
    case 'f':
        feedCat();
        break;
    case 'c':
        cleanCat();
        break;
        case 'h':
         checkHappiness();
         break;
    case '?':
        showHelp();
        break;
    case 'e':
        exit();
    default:
        cout << "That's not a command -yet (probably)" << endl;
    }
}

void feedCat() {
    cout << "Time for food!!!" << endl;
    cat.feed();
}

void cleanCat() {
    cout << "Let's take a bath!!!" << endl;
    cat.clean();
}

void checkHappiness(){
 cat.checkHappiness();
 }

int exit() {
    exit(0);
    return 0;
}

pet game.h:

    #ifndef PET_GAME_H_
#define PET_GAME_H_

#include <iostream>
#include "Cat.h"
//#include "User.h"

using namespace std;

//Global variables
char command;
int exit();

//functions
void commandIdentify();
void buyNextCat();
void showHelp();
void tutorial();
void feedCat();
void cleanCat();
void checkHappiness();

//functions defined for a future use but they cause problems right now
void checkMoney();
void checkLevel();


#endif /* PET_GAME_H_ */

Cat.cpp:

#include <iostream>
#include <string>
#include "Cat.h"

using namespace std;

Cat::Cat(){
    happiness = 1;
    cleanness = 1;
}

void Cat::feed(){
    cout << "Yummy!!" << endl;
    Cat::happiness = Cat::happiness + 0.2;
}

void Cat::clean(){
    cout << "So soupy!!" << endl;
    Cat::cleanness = Cat::cleanness + 0.2;
}

Cat.h:

#ifndef CAT_H_
#define CAT_H_

#include <iostream>

using namespace std;

class Cat {
private:
    int happiness;
    bool happy = happiness >= 0.6;

    int cleanness;
    bool isClean = cleanness >= 0.6;
public:
    Cat();
    void feed();
    void clean();
    void checkHappiness() {
        cout << "Your cat's happiness is " << happiness*100 << "%" << endl;
    }

};

#endif /* CAT_H_ */

it will be great if you could tell me how to improve code

New contributor
Ahmed Athir is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
1
  • \$\begingroup\$ For a C++ beginner, this is an excellent attempt. \$\endgroup\$ – indi Feb 24 at 0:51
2
\$\begingroup\$

Welcome to C++! You're off at a good start. Your code is very good for a beginner and it was very easy for me to read.


  • using namespace std

There's a mistake that I've seen most beginners make, (I'm guilty of it too) which is using namespace std.

There has been a long discussion about this on the stack overflow and I suggest you read it. In fact I've said this before multiple times in my reviews. The link I provided has excellent answers depicting the harmful effects of having this.

Namespaces in C++ are a way preventing name collisions in C++. to The std namespaces has tons of identifiers, tons. By doing using namespace std, you erase the important line that seperates your stuff and the std stuff.

namespace Apple {
  int a = 5;
}

namespace Orange {
  int a = 6;
}

int main() {
  std::cout << Orange::a << std::endl; // đź‘Ť 'a' in namespace Orange
}
namespace Apple {
  int a = 5;
}

namespace Orange {
  int a = 6;
}

int main() {
  using namespace Orange;
  using namespace Apple;
  std::cout << a << std::endl; // ?!?!?!?!?
}

in addition to this, when i see std::cout << a I want to know which a is being referenced here, any external library? A local variable? A global variable?

  • Moral of the story: Try to avoid doing this in your projects. Along the same lines, never have a using namespace in a header file. This header file gets included in other files and if you make it a habit you'll have no idea from where you're getting sudden name collisions.

  • Initializing members

Cat::Cat(){
    happiness = 1;
    cleanness = 1;
}

Instead of having this constructor, you can directly do

class Cat {
private:
  int happiness = 1;
  int cleanness = 1;
  Cat() = default;
  // ...

And btw, the word you're want is cleanliness :)

Since you are a beginner, I recommend watching this video on C++ constructors to get some extra knowledge


  • Global variables

You have some global variables in your program, and I have a problem with that. Put in other words, you have some variables with no scope in your program. That's exactly the problem.

//Global variables
char command;

I don't want to repeat myself here, but -> you will realize why its bad as the size of your codebase grows. The variable command is only needed by a few functions. There is no reason to have it leak into other scopes right? Given that command isn't a very distinctive name, if you by chance have to take in another input in another function, what do you do? command_2? These are some of the problems cause by globals. There are cases where you NEED them but this is surely not one of them.

Instead of creating a global variable, give it a scope and use function parameters to pass it around. Trust me when I say its much better to have that!


Other suggestions

  • unnecessary Cat::
void Cat::feed(){
    cout << "Yummy!!" << endl;
    Cat::happiness = Cat::happiness + 0.2;
}

void Cat::clean(){
    cout << "So soupy!!" << endl;
    Cat::cleanness = Cat::cleanness + 0.2;
}

Drop the Cat:: here, you're defining a member function you don't need it.

  • #pragma once
// Header file
#ifndef foo
#define foo 
// your code ...
#endif

You probably know what these are, (you've used them in your program). There is a nicer (although less portable) way of achieving the same which is to use #pragma once

#pragma once
// your code ...
\$\endgroup\$
5
  • \$\begingroup\$ #pragma once is only “nicer” when it works; when it fails, it’s an unholy nightmare to diagnose or fix… which is why it’s never been standardized. It’s bad advice to tell new programmers to use it. Even the core guidelines recommend against it. \$\endgroup\$ – indi Feb 25 at 1:39
  • \$\begingroup\$ @indi Oh, I've never heard anything about that. Why would it fail? \$\endgroup\$ – Aryan Parekh Feb 26 at 6:18
  • \$\begingroup\$ There are dozens of ways; far more than could be explained here. But two common classes of errors are: 1) the “same” file is accessed from different filesystems, which might happen if libraries are on network drives; and 2) a file is copied/modified by your build system so it’s no longer the “same” file, which might happen if a file is preprocessed somehow, or even just copied from the source dir to the build dir. These errors are functionally impossible to predict, may be silent, and are non-deterministic (may happen on Tuesday, but not Thursday). These are the worst kinds of errors. \$\endgroup\$ – indi 2 days ago
  • \$\begingroup\$ By contrast, the only way include guards can “break” is if you use the same token in two different headers… which is such a trivial and obvious problem that someone that learned C++ 10 minutes ago can spot it (and fix it!), or it can even be detected by a simple tool. And, of course, it can be mostly avoided in the first place by following simple guidelines. This is the best kind of error. \$\endgroup\$ – indi 2 days ago
  • \$\begingroup\$ Granted, it’s very rare that #pragma once will fail… but when it does, you’re just totally boned; like, there’s just no fixing it, you basically have to give up and rethink your entire project structure from scratch. But it’s still better than nothing, so if someone’s using nothing, then suggesting #pragma once is… okay-ish (still better to tell them to use include guards, though). But if someone’s already using *the correct thing*—include guards—telling them to switch to #pragma once is irresponsible, terrible advice. \$\endgroup\$ – indi 2 days ago
2
\$\begingroup\$
  • Avoid using namespace std; - it can cause problems with name collisions. It's best to qualify names where necessary, e.g.: std::cout

  • std::endl adds a newline, but it also flushes the output stream. Unless we actually need the flushing behavior, it's best to simply output a newline "\n" where necessary (e.g. << "2. to clean your cat, type c\n").

  • char command; does not need to be a global variable. It's only accessed in a single function, so it can be a local variable.

  • The exit() function is not necessary. We can call std::exit(0) directly. Note that we should include <cstdlib> for this function, and use the exit codes EXIT_SUCCESS or EXIT_FAILURE instead of a number like 0.

  • Functions that are used only in a single file do not need to be declared in header files. If necessary, we can declare them above the point of use, or move the function definition higher up the file. e.g.:

    void commandIdentify(); // forward declaration
    
    int main() { ... }
    
    void commandIdentify() { ... } // function definition
    

    or

    void commandIdentify() { ... } // function definition
    
    int main() { ... }
    

  • In the Cat class, the happy and isClean members are not quite correct. They will only be set on initialization. If the cat's happiness or cleanness changes at a later time, they will not be updated. In addition, initialization of variables occurs before the body of the constructor, in the order that they are declared in the class.

    For the initial values of happy and isClean to be set correctly, we would have to use a member initializer list:

    Cat::Cat():
        happiness(1),
        /* `happy` is initialized here */
        cleanness(1),
        /* `isClean` is initialized here */
    {
        // if we set happiness now, `happy` won't get updated!
        happiness = 0.2,
    }
    

    In any case, since we want the values to always be updated, we should make happy and isClean member functions instead:

    bool isClean() const { return cleanness >= 0.6; }
    

    That way, we don't need to update any extra state.


  • Member functions that don't modify the state of the class (i.e. any member veriables) should be declared const, e.g.:

    void checkHappiness() const { ... }
    

  • Cat cat; This doesn't need to be a global variable. We can make the cat a local variable in main() and pass a reference to it to the functions that need to use it:

    int main() {
        Cat cat;
        ...
        commandIdentify(cat);
    }
    
    void commandIdentify(Cat& cat) {
        ...
        feedCat(cat);
    }
    
    void feedCat(Cat& cat) {
        ...
        cat.feed();
    }
    

    This may seem like more work, but it's quite important, as means that our commandIdentify function (and the functions for each command) will now work with any cat, not just the one cat that we declared in main().

    So now we can feed many cats!

\$\endgroup\$

Your Answer

Ahmed Athir is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.