6
\$\begingroup\$

Here is the code for the game :

#include <iostream>
#include <sstream>
#include <string>
#include <cstdlib>
#include <ctime>
#include <algorithm>
#include <vector>

using std::vector;
using std::cout;
using std::cin;
using std::endl;
using std::time;
using std::stringstream;
using std::string;
using std::to_string;
const string bot = "B";

void riftmaker(int coordx, int coordy , vector<vector<string>>& rboard);
void infinity_ikea(const vector<vector<string>>& rboard);
void introduction(const vector<vector<string>>& rboard);
bool legal(const vector<vector<string>>& rboard , int choice , string skin);
bool wincon(const vector<vector<string>>& rboard , int coordx , int coordy);
void COMMANDO(vector<vector<string>>& rboard, int choice, string skin, string command , string player);
void pepegaheadqurters(int coordx ,int  coordy , vector<vector<string>>& rboard);
void error();
int skynet(vector<vector<string>>& rboard, string skin , int limit , int coordx , int coordy);

int main()
{
    //elements
    int coordx, coordy;
    srand(static_cast<unsigned int>(time(0)));
    //COORDS
    cout << "enter horizntal size :";
    cin >> coordx;
    cout << "enter vertical size :";
    cin >> coordy;
    vector<vector<string>> board;
    riftmaker(coordx, coordy, board);

    //INTRODUCTIONS
    introduction(board);
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    cin.get();
    //main
    pepegaheadqurters(coordx, coordy, board);
}

void riftmaker(int coordx , int coordy , vector<vector<string>>& rboard)//VECTOR ASSIGNER
{
    int boardnumber = 1;
    for (int i = 0; i < coordx; i++)
    {
        vector<string> row;
        for (int j = 0; j < coordy; j++)
        {
            row.push_back(to_string(boardnumber));
            boardnumber++;
        }
        rboard.push_back(row);
    }

}

void infinity_ikea(const vector<vector<string>>& rboard) //PRINTER
{
    int numberl;
    for (vector<string> loop : rboard) {
        for (string x : loop) {
            stringstream geek(x);
            geek >> numberl;
            cout << x;
            if (numberl < 10) {
                cout << " | ";
            }
            else if (numberl < 100 || numberl > 9) {
                cout << "| ";
            }
            else if (numberl == 0) {
                cout << " | ";
            }
            else {
                cout << " | ";
            }
        }
        cout << "\n";
    }
}
void introduction(const vector<vector<string>>& rboard) // INTRO
{
    cout << "\t\t4-CONNECT\n\t-----------------------\n\n\n";
    cout << "WELCOME TO 4-CONNECT U KNOW THE RULES AND THE SHT" << endl;
    cout << "the board :" << endl;
    infinity_ikea(rboard);
}

void pepegaheadqurters(int coordx, int  coordy, vector<vector<string>>& rboard) //MAIN FUNCTION
{
    //elements
    int limit = coordx * coordy;
    int blank = 0;
    int boardnumber, loobyposition;
    string skin, roboskin;
    roboskin = 'B';
    int robot;
    int i = 0;
    enum wincondition
    {
        win, lose, tie
    };
    wincondition w = win;


    //main body
    cout << "which letter will u use :";
    cin >> skin;
    cout << "\ndo u want to start first(0) or machine(1) :";
    cin >> loobyposition;
    error();


    //game loop
    while (blank != limit && wincon(rboard, coordx, coordy) != true)
    {
        if (loobyposition % 2 == 0) {
            infinity_ikea(rboard);
            cout << "enter number pls :";
            cin >> boardnumber;
            boardnumber -= 1;
            COMMANDO(rboard, boardnumber, skin, "MOVE", "human");
            wincon(rboard, coordx, coordy);
            error();
            infinity_ikea(rboard);
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            cin.get();
            error();
            blank++;
            loobyposition++;
            if (wincon(rboard , coordx , coordy)) {
                w = win;
            }
        }
        else if (loobyposition % 2 != 0 || wincon(rboard, coordx, coordy) != true) {
            robot = skynet(rboard, skin, limit, coordx, coordy);
            infinity_ikea(rboard);
            cout << "my number is :" << robot << endl;
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            cin.get();
            error();
            loobyposition++;
            blank++;
            if (wincon(rboard , coordx , coordy)) {
                w = lose;
            }

        }
    }
    infinity_ikea(rboard); 

    cout << "\n" << endl;
    if (blank == limit && wincon(rboard, coordx, coordy) != true) {
        w = tie;
    }
    if (w == win) {
        cout << "congrats u won";
    }
    else if (w == lose) {
        cout << "congreat u lose";
    }
    else {
        cout << "TIE";
    }
}




void error() // NEW PAGE
{
    for (int  i = 0; i < 1000; i++)
    {
        cout << "\n\n";
    }
}
bool wincon(const vector<vector<string>>& rboard, int coordx, int coordy) //WIN CONDITIONS
{
    bool check = true;
    int equ1, equ2;
    for (int x = 0; x < coordx; x++)
    {
        for (int y = 0; y < coordy; y++)
        {
            string startvalue = rboard[x][y];
            int possiblex[4][2] = { {0 , 1} , {1 , 0} , {1 , -1} , {1 , 1} };

            //checkloop
            for (int j = 0; j < 4; j++)
            {
                check = true;
                int i = 0;
                for (int b = 1; b < 4; ++b) {
                    equ1 = (x + (b * possiblex[j][i]));
                    equ2 = (y + (b * possiblex[j][i + 1]));
                    if (equ1 < 0 || equ1 == coordx) {
                        check = false;
                        break;
                    }
                    else if (equ2 < 0 || equ2 == coordy) {
                        check = false;
                        break;
                    }
                    else
                    {
                        if (rboard[equ1][equ2] != startvalue) {
                            check = false;
                        }
                    }
                }
                if (check == true) {
                    return check;
                }

            }
        }
    }
    return check;
}



void COMMANDO(vector<vector<string>>& rboard , int choice , string skin , string command , string player) //MOVE AND DELETE
{
    int counter = 0;
    int cols = 0;
    vector<vector<string>>::iterator raw;
    vector<string>::iterator col;

    for (raw = rboard.begin(); raw != rboard.end(); raw++) {
        for (col = raw->begin(); col != raw->end(); col++){
            if (counter == choice) {
                if (command == "MOVE") {
                    counter += raw->size();
                    while (raw != (rboard.end() - 1) && legal(rboard, counter, skin) == true)
                    {
                        raw++;
                        col = raw->begin() + cols;
                        counter += raw->size();
                    }
                    if (player == "human") {
                        *col = skin;
                    }
                    else
                    {
                        *col = bot;
                    }
                }
                else if (command == "DELETE") {
                    while (raw != (rboard.end() - 1) && legal(rboard, counter, skin) == true)
                    {
                        raw++;
                        col = raw->begin() + cols;
                        counter += raw->size();
                    }
                    if (*col == skin || *col == bot) {
                        *col = to_string(counter+1);
                    }
                }
            }
            counter++;
            cols++;
        }
        cols = 0;
    }
}



bool legal(const vector<vector<string>>& rboard , int choice ,string skin ) //LEGAL OR NOT
{
    int counter = 0;
    for (vector<string> loop : rboard) {
        for (string x : loop) {
            if (choice == counter){
                if (x == skin || x == bot) {
                    return false;
                }
            }
            counter++;
        }
    }
    return true;
}
int skynet(vector<vector<string>>& rboard, string skin, int limit, int coordx, int coordy) //AI
{
    //checkwin and stop win
    int counter = 0;
    while (counter < limit) {
        if (legal(rboard, counter, skin)) {
            //check win
            COMMANDO(rboard, counter, skin, "MOVE", "bot");
            if (wincon(rboard, coordx, coordy) == true) {
                return counter + 1;
            }
            COMMANDO(rboard, counter, skin, "DELETE", "bot");
            //check enemy
            COMMANDO(rboard, counter, skin, "MOVE", "human");
            if (wincon(rboard, coordx, coordy) == true) {
                COMMANDO(rboard, counter, skin, "DELETE", "human");
                COMMANDO(rboard, counter, skin, "MOVE", "bot");
                return counter + 1;
            }
            COMMANDO(rboard, counter, skin, "DELETE", "human");

        }
        counter++;
    }
    //random number generatror
    for (int i = 0; i < 1000; i++)
    {
        counter = rand() % limit;
        if (legal(rboard, counter, skin)) {
            COMMANDO(rboard, counter, skin, "MOVE", "bot");
            return counter + 1;
        }
    }
}

What could I have done better with this code, how could I make it shorter.

I know some functions have childish names, but I wrote their function near their definitions.

I overcomplicated stuff with the move and delete function and wincondition function, could I make it easier or not, or it's good.

\$\endgroup\$

2 Answers 2

9
\$\begingroup\$

I think there's quite a lot to improve, especially the first point I mention below. Consider updating your code according to the suggestions and then creating a new review question on Code Review.

Use proper names and spelling

I know some functions have childish names, but [...]

The names are indeed childish. Sometimes it's fun to do this, but the consequence is that the code is now much harder to read. If you give a function or variable a name that clearly and concisely says what the function is supposed to do or represent, it is much easier to understand your source code. Not only is that important for others, but also for your future self! In half a year you will have forgotten the details, and then this code will look unreadable to you.

I also recommend you use proper spelling, grammar and use consistent typography, and keep things professional. So instead of:

cout << "WELCOME TO 4-CONNECT U KNOW THE RULES AND THE SHT" << endl;
cout << "the board :" << endl;

Write:

cout << "Welcome to Connect Four! The usual rules apply." << endl;
cout << "The board:" << endl;

Write "you" instead of "u", don't add a space before a colon, start sentences with a capital and end them with some kind of punctuation. Basically, write like you find text written in a book or on a professional website.

Maybe this takes the fun out of things, but consider it good practice for your future career, and also be aware that what you post now on the Internet might stay there for a long time for everyone to see!

Avoid many using std::s

It's generally considered bad practice to use using namespace std. You did not do that, but instead you have a whole list of using std::somethings, in effect just being a more verbose form of using namespace std.

Use \n instead of std::endl

Prefer writing \n instead of using std::endl, the latter is equivalent to the former, but also forces the output to be flushed, which is usually not necessary and just leads to worse performance.

Use C++11's random number generators

The srand() and rand() functions come from C, and are of low quality. Consider using the C++11 random number functionality. See this StackOverflow post for how to use it.

Create an enum class representing a cell on the board

Each cell on the board can be empty, have a piece of the human player, or that of the bot. Using a std::string for each cell is quite inefficient, and it's easy to make mistakes. Use an enum class to define what the possible states of each cell are:

enum class Cell {
    EMPTY,
    HUMAN,
    BOT
};

Then declare your board as:

std::vector<std::vector<Cell>> board;

Instead of writing board[x][y] = skin, you would now write board[x][y] = Cell::HUMAN. Apart from being more efficient, the benefit is also that the compiler will be able to catch certain mistakes. For example, if you assigned any other string to a cell besides skin or roboskin, you would only notice the mistake when the program was not behaving as expected. However, with an enum class, the compiler would complain about anything that wasn't defined earlier in enum class Cell.

Consider creating a class representing the board

You can encapsulate all the properties of a board in a class. You can start simple by just having it be a wrapper around the vector of vectors:

class Board {
public:
    std::vector<std::vector<Cell>> cells;
};

And now in main(), you can write:

Board board;

And pass it around to all the other functions that want a board. This already avoids you having to write std::vector<std::vector<std::string>> &rboard, it can now just be Board &rboard.

The next step is to make the functions manipulating the board member functions of class Board. This allows them easy access to the cells. For example:

class Board {
public:
    std::vector<std::vector<Cell>> cells;
    bool legal(int choice, Cell player) const;
};

bool Board::legal(int choice, Cell player) const {
    int counter = 0;
    for (vector<string> loop : cells) {
        ...
    }
    ...
}

And then it can be called from "skynet()" as follows:

if (rboard.legal(counter, player)) {
    ...
}

Avoid using vectors of vectors

A vector of vectors is an inefficient data structure. Although for a simple game like connect four, it's not going to be an issue. Unfortunately, there's no nice 2D array type in the standard library, although there many external libraries that provide them. The standard trick however is to just use a single std::vector, give it as many elements in total as the original vector of vectors, and calculate the index into the one-dimensional vector. If you have a grid of nrows by ncols elements, then instead of writing:

std::vector<std::vector<...>> cells;
...
cells[x][y] = ...;

You would write:

std::vector<...> cells;
...
cells[x + y * ncols] = ...;
\$\endgroup\$
12
  • \$\begingroup\$ Can u care to explain the last part in your answer, I can't seem to understand the equation at the end, and it the program, and I didn't use classes because my book didn't reach this part , but when the book reaches this part, I will be sure to use classes and post my code here. \$\endgroup\$ May 25, 2021 at 0:14
  • \$\begingroup\$ here is the equation which I can't understand:cells[x + y * ncols] = ...; \$\endgroup\$ May 25, 2021 at 0:23
  • \$\begingroup\$ I understood it now, u mean cells[(x + ncols) + y] = ...; \$\endgroup\$ May 25, 2021 at 0:27
  • \$\begingroup\$ If you have a vector of length nrows * ncols, and x is representing the column and y the row, and assume first all the elements of the first row are stored in the vector, then all the elements of the second row and so on, then the start of each row in the vector is at offset ncols * y (because each row has ncols elements). Then we just add x to get to the right element within that row. \$\endgroup\$
    – G. Sliepen
    May 25, 2021 at 6:59
  • \$\begingroup\$ ok, ty for your information @G. Sliepen \$\endgroup\$ May 25, 2021 at 13:42
3
\$\begingroup\$
wincon(rboard, coordx, coordy) != true

That's silly. Just write !wincon(rboard, coordx, coordy) for that term.

    if (w == win) {
        cout << "congrats u won";
    }
    else if (w == lose) {
        cout << "congreat u lose";
    }
    else {
        cout << "TIE";
    }

Since you are checking the various values of an enumeration type, use a switch statement.

And, I reiterate the point about spelling and the awful use of "u" for "you". When I read that I imagine /ʌ/ (like you started to say "up") rather than /ju/

    for (vector<string> loop : rboard) {
        for (string x : loop) {

Do you realize that you are copying by value the vector of strings in the outer loop and the string in the inner loop? You should be using const & and best to use auto for the iteration variable because if it doesn't match the type exactly you can get extra conversions and copies without any error.


Just what is it doing? It parses each string in the vector as a number... why not just store the numbers directly? And then it just prints spaces and pipe character depending on the range of the number (how many digits it has?). I have no idea what the point of this is, and the goofy names don't help.

Edit: Ah, I see it prints the string first, then this adds trailing spaces plus the (unchanging) pipe and another space. You just want to print x justified to the full width. Look at the ostream features that do that automatically, such as the setw manipulator.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ thank you for pointing things out, I will use them. \$\endgroup\$ May 25, 2021 at 20:30

Your Answer

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.