Ciężko powiedzieć od czego zacząć w ogóle.
-
player.h i pewnie nie tylko - masz losowe wcięcia. Powinno być jednolicie.
-
Po co ci w ogóle pusty destruktor w player.h? W ogóle po co jest ta klasa? Nie wystarczyłoby coś takiego (przy okazji pozbywając się obrzydliwej duplikacji w przypadku Opponent)?
enum class PlayerType {
MainPlayer,
Opponent
}
struct Player {
const string name;
const int age;
const PlayerType type;
void printInfo(); // notabene, to printInfo nie powinno przyjmować dodatkowych parametrów
};
a potem używane o tak:
Player p = {"abc", 27, PlayerType::MainPlayer};
p.name;
p.age;
p.printInfo();
-
Masz taki kod
if (numberRepeats(arr)==4) {
std::random_device rd; // Inicjalizacja generatora losowego
std::mt19937 gen(rd()); // Inicjalizacja generatora Mersenne Twister
std::uniform_int_distribution<int> diceRoll(1, 6); // Zakres liczb losowych od 1 do 6
arr[findDifferentPosition(arr)] = diceRoll(gen);
return 0;
}
// a potem
if (twoPairs(arr) == true) {
std::random_device rd; // Inicjalizacja generatora losowego
std::mt19937 gen(rd()); // Inicjalizacja generatora Mersenne Twister
std::uniform_int_distribution<int> diceRoll(1, 6); // Zakres liczb losowych od 1 do 6
arr[findLeastFrequentIndex(arr)] = diceRoll(gen);
return 0;
}
to aż się prosi do wydzielenia do osobnej funkcji.
-
Wiesz co to jest pętla?
sleep(1);
std::cout << "3.." << std::endl;
sleep(1);
std::cout << "2.." << std::endl;
sleep(1);
std::cout << "1.." << std::endl;
sleep(1);
std::cout << "Let's roll the dice" << std::endl;
-
if (warunek) return true; return false
to to samo co return warunek;
. Poza tym, zupełnie nie jest czytelne co ta funkcja ma w zamierzeniu robić. Jak masz jakieś skomplikowane warunki, przynajmniej dodaj komentarz opisujący co to robi.
bool twoPairs(int a1[]) {
if (((a1[0] == a1[1] && a1[2] == a1[3]) || (a1[0] == a1[2] && a1[1] == a1[3]) || (a1[0] == a1[3] && a1[1] == a1[4])) || (a1[1] == a1[2] && a1[3] == a1[4]) || (a1[0] == a1[1] && a1[3] == a1[4]))
return true;
return false;
}
-
Funkcja winLose
jest obrzydliwa. Nawet nie wiem co dokładnie ma robić, ale przemyśl jeszcze raz co to miało robić, i wyraź prościej te warunki. Być może z pomocą pętli (bo wygląda na to, że chociaż jedna tam by się mogła zdarzyć).
-
Błędna indentacja tutaj: powinieneś zwiększyć wcięcie w środku bloku do while:
do {
while (true) {
std::cout << "Choose difficulty level: 1 - easy, 2 - hard" << std::endl;
std::cin >> difficultyLevel;
if (difficultyLevel == 1 || difficultyLevel == 2) {
break;
} else {
std::cout << "Wrong number! Try again!" << std::endl;
}
}
std::cout << "The player rolls the dice" << std::endl;
sleep(1);
-
int menu() {
-> zrób na to enum
a, bo zupełnie nie jest potem jasne co te wartości znaczą.
-
void makePlayer (std::string &playerName, int &age, char &gender) {
, wbrew temu co nazwa obiecuje, wcale nie tworzy żadnego gracza. Sobie pobiera jakieś wartości, coś tam wypisuje, zapisuje gdzieś wartości, ale wcale gracza nie tworzy. Albo zwracaj gracza (więc nie void makePlayer), albo chociaż przyjmuj Player&
jako argument.
-
Zdecyduj się, czy używasz rand()
, czy std::random_device
srand(time(NULL));
int drawRandomWord = rand() % 20;
-
Jak nie chcesz tutaj warninga na porównanie zmiennych różnych typów, lepiej zrób size_t i = 0
zamiast tego rzutowania potem.
for (int i = 0; i < int(drawnWord.length()); ++i)
-
Znów, wiesz czym jest pętla? A poza tym, isSomeoneWon
to niezbyt gramatycznie poprawnie.
bool isSomeoneWon(char (&arr)[9], int& bankrollTictactoe) {
if ((arr[0] == 'X' && arr[1] == 'X' && arr[2] == 'X') ||
(arr[3] == 'X' && arr[4] == 'X' && arr[5] == 'X') ||
(arr[6] == 'X' && arr[7] == 'X' && arr[8] == 'X') ||
(arr[0] == 'X' && arr[3] == 'X' && arr[6] == 'X') ||
(arr[1] == 'X' && arr[4] == 'X' && arr[7] == 'X') ||
(arr[2] == 'X' && arr[5] == 'X' && arr[8] == 'X') ||
(arr[0] == 'X' && arr[4] == 'X' && arr[8] == 'X') ||
(arr[2] == 'X' && arr[4] == 'X' && arr[6] == 'X')) {
std::cout << "Game over. You win!" << std::endl;
bankrollTictactoe+=5;
return true;
}
-
if (whoBegin())
, to po pierwsze. Po drugie, czemu ewaluujesz whoBegin
dwukrotnie? Po trzecie, tam nie ma więcej niż dwóch opcji, a więc zamiast else if
powinno być else
.
if (whoBegin() == true) {
// ...
}
else if (whoBegin() == false) {
// ...
}
-
Nie mieszaj języków. Czemu cały Makefile jest po polsku?
Więcej uwag będzie, jak już z tymi się uporasz.