• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Bardziej "czysta" wersja kodu

VPS Starter Arubacloud
0 głosów
226 wizyt
pytanie zadane 7 sierpnia 2018 w C i C++ przez Huberti Gaduła (4,500 p.)
edycja 7 sierpnia 2018 przez Huberti

Witam, która z poniższych wersji kodu jest bardziej "czysta"? Przy okazji proszę rownież o ocenę kodu. 

Jest to plik GameModes.cpp z gry kółko i krzyżyk. Wrzuciłem cały, aby lepiej zobrazować sytuację.

Wersja 1 (kod bezpośrednio w funkcji, co powoduje usunięcie podwójnego sprawdzenia czyja jest teraz tura):

#include "GameModes.h"
#include "TicTacToe.h"
#include "ManagingNamesOfPlayers.h"

resultOfGame makePlayerVsComputerGame(const std::string &nameOfPlayer)
{
	char gameBoard[9]{ '1', '2', '3', '4', '5', '6', '7', '8', '9' };
	std::string whoseTurn = getRandomlyWhoStartsGame();
	int indexOfCellWithoutProprietorOfGameBoard;

	system("cls");
	if (isCircleTurn(whoseTurn))
	{
		std::cout << "Twoja tura " << nameOfPlayer << " (O)" << std::endl;
		showGameBoard(gameBoard);
		indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
	}	
	else
	{
		std::cout << "Tura przeciwnika (X)" << std::endl;
		showGameBoard(gameBoard);
		indexOfCellWithoutProprietorOfGameBoard = getRandomIndexOfCellWithoutProprietorOfGameBoard(gameBoard);
		std::cout << "Wybieram komorke z numerem " << indexOfCellWithoutProprietorOfGameBoard + 1 << std::endl;
		waitTwoSecounds();
	}
	setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);

	while (!isEndGame(gameBoard))
	{
		setTurnToTheNextPlayer(whoseTurn);
		system("cls");	
		if (isCircleTurn(whoseTurn))
		{
			std::cout << "Twoja tura " << nameOfPlayer << " (O)" << std::endl;
			showGameBoard(gameBoard);
			indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
		}		
		else
		{
			std::cout << "Tura przeciwnika (X)" << std::endl;
			showGameBoard(gameBoard);
			indexOfCellWithoutProprietorOfGameBoard = getRandomIndexOfCellWithoutProprietorOfGameBoard(gameBoard);
			std::cout << "Wybieram komorke z numerem " << indexOfCellWithoutProprietorOfGameBoard + 1 << std::endl;
			waitTwoSecounds();
		}
		setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);
	}

	system("cls");
	showGameBoard(gameBoard);
	if (isTie(gameBoard))
	{
		std::cout << "Remis.";
		return GAME_WITHOUT_WINNER;
	}		
	else if (isCircleTurn(whoseTurn))
	{
		std::cout << "Wygrana" << std::endl;
		return FIRST_PLAYER_WON_GAME;
	}		
	else
	{		
		std::cout << "Przegrana" << std::endl;
		return FIRST_PLAYER_LOST_GAME;
	}
}

resultOfGame makePlayerVsPlayerGame(const NamesOfPlayers &namesOfPlayers)
{
	char gameBoard[9]{ '1', '2', '3', '4', '5', '6', '7', '8', '9' };
	std::string whoseTurn = getRandomlyWhoStartsGame();
	int indexOfCellWithoutProprietorOfGameBoard;

	system("cls");
	if (isCircleTurn(whoseTurn))
		std::cout << "Tura gracza " << namesOfPlayers.nameOfFirstPlayer << " (O)" << std::endl;
	else
		std::cout << "Tura gracza " << namesOfPlayers.nameOfSecoundPlayer << " (X)" << std::endl;
	showGameBoard(gameBoard);
	indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
	setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);

	while (!isEndGame(gameBoard))
	{
		setTurnToTheNextPlayer(whoseTurn);
		system("cls");
		if (isCircleTurn(whoseTurn))
			std::cout << "Tura gracza " << namesOfPlayers.nameOfFirstPlayer << " (O)" << std::endl;
		else
			std::cout << "Tura gracza " << namesOfPlayers.nameOfSecoundPlayer << " (X)" << std::endl;
		showGameBoard(gameBoard);
		indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
		setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);
	}

	system("cls");
	showGameBoard(gameBoard);
	if (isTie(gameBoard))
	{
		std::cout << "Remis" << std::endl;
		return GAME_WITHOUT_WINNER;
	}		
	else if (isCircleTurn(whoseTurn))
	{
		std::cout << "Wygrywa gracz " << namesOfPlayers.nameOfFirstPlayer << std::endl;
		return FIRST_PLAYER_WON_GAME;
	}		
	else
	{
		std::cout << "Wygrywa gracz " << namesOfPlayers.nameOfSecoundPlayer << std::endl;
		return FIRST_PLAYER_LOST_GAME;
	}		
}

Wersja 2 (kod umieszczony w osobnej funkcji, co powoduje zmniejszenie funkcji, ale kosztem podwójnego sprawdzenia czyja jest teraz tura):

#include "GameModes.h"
#include "TicTacToe.h"
#include "ManagingNamesOfPlayers.h"

resultOfGame makePlayerVsComputerGame(const std::string &nameOfPlayer)
{
	char gameBoard[9]{ '1', '2', '3', '4', '5', '6', '7', '8', '9' };
	std::string whoseTurn = getRandomlyWhoStartsGame();
	int indexOfCellWithoutProprietorOfGameBoard;

	system("cls");
	showMessageWhoseTurnIsNow(whoseTurn, nameOfPlayer);
	showGameBoard(gameBoard);
	if (isCircleTurn(whoseTurn))
		indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
	else
	{
		indexOfCellWithoutProprietorOfGameBoard = getRandomIndexOfCellWithoutProprietorOfGameBoard(gameBoard);
		std::cout << "Wybieram komorke z numerem " << indexOfCellWithoutProprietorOfGameBoard + 1 << std::endl;
		waitTwoSecounds();
	}
	setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);

	while (!isEndGame(gameBoard))
	{
		setTurnToTheNextPlayer(whoseTurn);
		system("cls");
		showMessageWhoseTurnIsNow(whoseTurn, nameOfPlayer);
		showGameBoard(gameBoard);
		if (isCircleTurn(whoseTurn))
			indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
		else
		{
			indexOfCellWithoutProprietorOfGameBoard = getRandomIndexOfCellWithoutProprietorOfGameBoard(gameBoard);
			std::cout << "Wybieram komorke z numerem " << indexOfCellWithoutProprietorOfGameBoard + 1 << std::endl;
			waitTwoSecounds();
		}
		setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);
	}

	system("cls");
	showGameBoard(gameBoard);
	if (isTie(gameBoard))
	{
		std::cout << "Remis.";
		return GAME_WITHOUT_WINNER;
	}		
	else if (isCircleTurn(whoseTurn))
	{
		std::cout << "Wygrana" << std::endl;
		return FIRST_PLAYER_WON_GAME;
	}		
	else
	{		
		std::cout << "Przegrana" << std::endl;
		return FIRST_PLAYER_LOST_GAME;
	}
}

resultOfGame makePlayerVsPlayerGame(const NamesOfPlayers &namesOfPlayers)
{
	char gameBoard[9]{ '1', '2', '3', '4', '5', '6', '7', '8', '9' };
	std::string whoseTurn = getRandomlyWhoStartsGame();
	int indexOfCellWithoutProprietorOfGameBoard;

	system("cls");
	showMessageWhoseTurnIsNow(whoseTurn, namesOfPlayers);
	showGameBoard(gameBoard);
	indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
	setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);

	while (!isEndGame(gameBoard))
	{
		setTurnToTheNextPlayer(whoseTurn);
		system("cls");
		showMessageWhoseTurnIsNow(whoseTurn, namesOfPlayers);
		showGameBoard(gameBoard);
		indexOfCellWithoutProprietorOfGameBoard = getIndexOfCellOfGameBoardFromInputStreamWhileCellOfThatIndexHasProprietor(gameBoard);
		setCellOfGameBoardToBePropertyOfCurrentPlayer(gameBoard[indexOfCellWithoutProprietorOfGameBoard], whoseTurn);
	}

	system("cls");
	showGameBoard(gameBoard);
	if (isTie(gameBoard))
	{
		std::cout << "Remis" << std::endl;
		return GAME_WITHOUT_WINNER;
	}		
	else if (isCircleTurn(whoseTurn))
	{
		std::cout << "Wygrywa gracz " << namesOfPlayers.nameOfFirstPlayer << std::endl;
		return FIRST_PLAYER_WON_GAME;
	}		
	else
	{
		std::cout << "Wygrywa gracz " << namesOfPlayers.nameOfSecoundPlayer << std::endl;
		return FIRST_PLAYER_LOST_GAME;
	}		
}

Fragment z pliku TicTacToe.cpp

void showMessageWhoseTurnIsNow(const std::string &whoseTurn, const std::string &nameOfPlayer)
{
	if (whoseTurn == "circle")
		std::cout << "Twoja tura " << nameOfPlayer << " (O)" << std::endl;
	else
		std::cout << "Tura przeciwnika (X)" << std::endl;
}

void showMessageWhoseTurnIsNow(const std::string &whoseTurn, const NamesOfPlayers &namesOfPlayers)
{
	if (whoseTurn == "circle")
		std::cout << "Tura gracza " << namesOfPlayers.nameOfFirstPlayer << " (O)" << std::endl;
	else
		std::cout << "Tura gracza " << namesOfPlayers.nameOfSecoundPlayer << " (X)" << std::endl;
}
1
komentarz 7 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)

Nie znam się na C więc samego kodu nie ocenię, ale...

setCellOfGameBoardToBePropertyOfCurrentPlayer

to nie jest fajna nazwa... za długa... Owszem, należy tworzyć jasne i samokomentujące się nazwy funkcji, zmiennych itp. ale nie można przesadzać :)

Jeśli mimo wszystko wg Ciebie taka nazwa na prawdę ma sens aby wyrazić to co funkcja robi to zastanów się, czy nie lepiej rozbić tę funkcjonalność na mniejsze.

Poczytaj też np. o TDD, czyli zasadzie najpierw testy, potem kod. Nie chodzi o to abyś od razu wprowadzał to w życie, ale żebyś zaczął więcej myśleć o przyszłościowych testach co wpłynie na pewno na jakość Twojego kodu.

komentarz 8 sierpnia 2018 przez Huberti Gaduła (4,500 p.)

Czy istnieje sposób, aby rozbić tę funkcję na mniejsze?

void setCellOfGameBoardToBePropertyOfCurrentPlayer(char &cell, const std::string &whoseTurn)
{
	if (whoseTurn == "circle")
		cell = 'O';
	else
		cell = 'X';
}
2
komentarz 8 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)

Rozbić w sumie nie, ale nazwa na pewno jest zła... jak dojdziesz do końca to zapomnisz co było w nazwie na początku... a jak w kodzie będzie takich dużo to nawet z IDE i podpowiadaniem tego nie ogarniesz...

a tak w ogóle to nie lepiej to zapisać krócej jako:

cell = (whoseTurn == "circle") ? "O" : "X" 

 

komentarz 8 sierpnia 2018 przez Huberti Gaduła (4,500 p.)

Dzięki, faktycznie tak wygląda to lepiej.

Właściwie to można jeszcze napisać tak:

char getSymbolOfCurrentPlayer(const std::string &whoseTurn)
{
    return (whoseTurn == "circle") ? 'O' : 'X';
}

Wtedy wywołanie:

gameBoard[indexOfCell] = getSymbolOfCurrentPlayer(whoseTurn);
komentarz 8 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)
I to się nazywa Kolego refaktoryzacja kodu... tutaj zaczyna się wg mnie różnica między "gościem po bootcampie", a kimś, kto faktycznie chce być programistą co widać u Ciebie.

Także działaj dalej i na spokojnie poprzeglądaj swój kod bo widzę, że szybko załapiesz wiele dobrych praktyk i sam do wielu rzeczy dojdziesz :) Gdy to zrobisz to możesz też powoli wchodzić w zagadnienia np. wzorców projektowych i sam zobaczysz jak jeszcze można niektóre problemy zmodyfikować :)

Pozdrawiam i powodzenia!

Zaloguj lub zarejestruj się, aby odpowiedzieć na to pytanie.

Podobne pytania

0 głosów
2 odpowiedzi 340 wizyt
0 głosów
1 odpowiedź 639 wizyt
pytanie zadane 23 grudnia 2017 w C i C++ przez Huberti Gaduła (4,500 p.)
0 głosów
2 odpowiedzi 407 wizyt
pytanie zadane 19 października 2015 w HTML i CSS przez Javowiec Pasjonat (21,560 p.)

92,453 zapytań

141,262 odpowiedzi

319,086 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...