I'm trying to get hired by some company and they gave me a simple remote test to take at home so they can judge my coding skills.It's a really simple problem based on the website HackerRank. It's a mini game where you have to keep track of the score of a tennis game (0-15-30-40-ADV).This is really basic and I managed to do it without any problems. But I believe the more important thing is not if it works or not but rather how you did it and if you did code like a professional.What could I improve on in my code to actually code like a professional?//======================================// Author: Philippe Balleydier// Last Update : 08/09/16// Object : Little algorithm able to keep track of the tennis score//======================================#include <iostream>#include <stdlib.h>#include <time.h>#include <string>using namespace std;// score map for printing the scoreconst string Score[] = {0, 15, 30, 40, ADV};//======================================// Class Game: contains all the informations and methods about the game being played//======================================class Game{    // see below for explanations    public:    Game();    bool createPlayer(int playerId, string playerName);    string getName(int playerId);    bool hasADV(int playerId);    bool setScore(int playerId, int score);    int getIntScore(int playerId);    string getStringScore(int playerId);    private:    string players[2]; // players' name    int scores[2]; // players' score};//--------------------------------------// Constructor : init score to 0//--------------------------------------Game::Game(){    scores[0] = 0;    scores[1] = 0;}//--------------------------------------// createPlayer : create a player depending on its ID//--------------------------------------bool Game::createPlayer(int playerId, string playerName){    if (playerId == 0 || playerId == 1)    {        players[playerId] = playerName;        return true;    }    else        return false;}//--------------------------------------// getName : return the name of player playerId as a string//--------------------------------------string Game::getName(int playerId){    if (playerId == 0 || playerId == 1)        return players[playerId];    else        return N/A;}//--------------------------------------// getName : return true if the player's score is ADV, else false//--------------------------------------bool Game::hasADV(int playerId){    if (scores[playerId]==4)        return true;    else        return false;}//--------------------------------------// setScore : change the score of player playerId//--------------------------------------bool Game::setScore(int playerId, int score){    if (score < 5)        scores[playerId]=score;    else        return false;}//--------------------------------------// getIntScore : return the score of player playerId as a int (0, 1, 2, 3, 4)//--------------------------------------int Game::getIntScore(int playerId){    return scores[playerId];}//--------------------------------------// getStringScore : return the score of player playerId as a string (0, 15, 30, 40, ADV)//--------------------------------------string Game::getStringScore(int playerId){    return Score[scores[playerId]];}//=======================================int main(){    // rand init    srand(time(NULL));    // setting up the game    Game myGame;    myGame.createPlayer(0, John);    myGame.createPlayer(1, Paul);    //starting game loop    bool stop = false;    while(!stop)    {        // identifying winner and looser        int pointWinner = rand()%2;        int pointLooser=(pointWinner+1)%2;        cout << The winner of the point is  << myGame.getName(pointWinner) << endl;        // The pointWinner has advantage => win        if (myGame.hasADV(pointWinner))        {            stop = true;            cout << Winner is  << myGame.getName(pointWinner) << endl;        } // The opponent has advantage => tie (40-40)        else if (myGame.hasADV(pointLooser))            myGame.setScore(pointLooser, 3);        // Tie (40-40) => pointWinner gets advantage        else if (myGame.getIntScore(pointWinner)==3 && myGame.getIntScore(pointLooser) == 3)            myGame.setScore(pointWinner, 4);        //pointWinner is 1 point ahead of pointLooser and >=30        else if (myGame.getIntScore(pointWinner)>myGame.getIntScore(pointLooser) && myGame.getIntScore(pointWinner)>=2)        {            stop = true;            cout << Winner is  << myGame.getName(pointWinner) << endl;        }        else // nothing special, pointWinner marks the point            myGame.setScore(pointWinner, myGame.getIntScore(pointWinner)+1);        // printing score        if (!stop)            cout << Score :  << myGame.getName(0) <<   << myGame.getStringScore(0) <<  -  << myGame.getStringScore(1) <<   << myGame.getName(1) << endl;    }    return 0;}Just so you know, it has to be a 1-page .cpp file, so I can't really split the Game class into different .h and .cpp files.
Keeping track of the tennis score
c++;interview questions
OverallYou missed the point.If you are supposed to create a class to track the score of game you did it wrong. Most of the logic that should be inside the class has been left inside main.If you see the pattern:Get info from object.Manipulate data.Update state of object.This usually means that you should have a function called Manipulate() (where Manipulate is the action you are doing).I would expect the logic running the game to be:while(game){     game.winPoint(rand() % 2);     std::cout << game << \n;}std::cout << Winner was:  << game.winner() << \n;Thus your interface to the class should be:class Game{    public:        // returns true if the game is still being played        bool operator() const;        // A player won a point (updates the state.        void winPoint(int playerId);        // return a reference to the winning player.        // If the game is not finished calling this is UB.        Player& winner() const;        // Print the current state of the game.        void print(std::ostream& str) const;        friend std::ostream& operator<<(std::ostream& s, Game const& g)        {            g.print(s);            return s;        }};Code ReviewComments.Comments should be meaningful.  A bad comment is worse than no comment. This is because like code, comments need to be maintained. If the comments and code fall out of sync then what does a maintainer do? Fix the code or fix the comment?So comments should be for describing why (not how).The first two lines are OK.//======================================// Author: Philippe Balleydier// Last Update : 08/09/16// Object : Little algorithm able to keep track of the tennis score//======================================The trouble is the third line is not accurate. It's not an algorithm it's a class.This comment on the other hand is completely useless. It tells me nothing that I don't already know.//======================================// Class Game: contains all the informations and methods about the game being played//======================================Header files.These are C header files.#include <stdlib.h>#include <time.h>There are C++ versions of these header files. The C++ versions put all the appropriate declarations in the namespace std.#include <cstdlib>#include <ctime>Never Do this.using namespace std;If you had read any other C++ review on this board it would have told you not to do this. See: Why is using namespace std in C++ considered bad practice?.There is a reason that the standard libraries are called std and not standard. Its to make prefixing them easy.This should be a private member of the class.// score map for printing the scoreconst string Score[] = {0, 15, 30, 40, ADV};Better indentingNice indenting is critical to coding. The compiler does not care. But the code will have to be read by humans long after you abandon this code to some poor maintainer. So please make it easy to read.    class Game    {        // see below for explanations        public:   =>   Game();   =>   bool createPlayer(int playerId, string playerName);I want to see that those methods are inside the public region of the class.ConstructorIs that comment telling me something I can not read in the code. Remove useless comment.   //--------------------------------------// Constructor : init score to 0//--------------------------------------Game::Game(){    scores[0] = 0;    scores[1] = 0;}The problem is that you have not set up the class completely. You have just created an empty object. You require the user of your class to make two further calls to set up the players before the class is ready to use. This means I can accidentally start using the class before it is completely set up.I would have done it like this: Player player1(Loki); Player player2(Thor); Game   game(player1, player2); // Game is now in a valid state and ready to go.Use exceptions when you have an errorbool Game::createPlayer(int playerId, string playerName)OK so I fail to add a player now what. The fact that you have an error condition probably means that there is a serious flaw in your code (the code that is calling the class) so your application should probably fail fast rather than try to continue.if (!myGame.createPlayer(2, Bob)){    // Failed to create player.    // what can I do here?}Also you return an error code but fail to even try and check it in your main application. That is a serious flaw (especially in interview code).myGame.createPlayer(0, John);  // returns an unchecked bool !!!!!myGame.createPlayer(1, Paul);Getter/Setter Bad practiceIt means you are leaking implementation details of the code. Prefer to use action methods that internally mutate the state of the object.Note: Getter/Setter are fine on object bags; but not on classes.Aside: Another bad comment (on each function). The comment is unneeded because the function is well named, so well that I can see exactly what it is doing without the comment.//--------------------------------------// getName : return the name of player playerId as a string//--------------------------------------string Game::getName(int playerId)    bool Game::hasADV(int playerId)bool Game::setScore(int playerId, int score)    int Game::getIntScore(int playerId)string Game::getStringScore(int playerId)Use the new random number    // rand init    srand(time(NULL));Sure it works. For this kind of example it is also fine. But this is an interview question. You want to show that you are up-to-date with modern C++ so use the new random number stuff.Mainmain() is special. If you don't return a value from main then the compiler will plant return 0; for you. So don't add a return to main.I use the fact that if there is a return 0; in my code that it indicates that at another point in main there is a return that is not zero.