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

Ocena kodu źródłowego

VPS Starter Arubacloud
+1 głos
198 wizyt
pytanie zadane 4 listopada 2018 w C i C++ przez Huberti Gaduła (4,500 p.)

Witam, proszę o ocenu kodu źródłowego. Ma on za zadanie przechowywanie informacji o tym, którego gracza jest obecnie tura. Czy takie testy jednostowe są wystarczające? Powinno się testować każdą metodę z osobna? Bo na przykład teraz metoda set pozostaje nieprzetestowana.

PS (namespace ttt pochodzi od TicTacToe).

WhoseTurn.h

#pragma once

namespace ttt {
	class WhoseTurn
	{
	public:
		enum class PlayerNumber { FIRST, SECOND };
		enum class ErrorType { BAD_PLAYER_NUMBER };

		WhoseTurn(const PlayerNumber& playerNumber = PlayerNumber::FIRST);

		void set(const PlayerNumber& playerNumber);

		void setTurnToNextPlayer();

		void setRandomly();

		bool isFirstPlayerTurn() const { return playerNumber == PlayerNumber::FIRST; };

		bool isSecondPlayerTurn() const { return playerNumber == PlayerNumber::SECOND; };

	private:
		PlayerNumber playerNumber;
	};
}

WhoseTurn.cpp

#include "WhoseTurn.h"
#include <ctime>
#include <cstdlib>

namespace ttt {
	WhoseTurn::WhoseTurn(const PlayerNumber& playerNumber)
	{
		if (playerNumber != PlayerNumber::FIRST && playerNumber != PlayerNumber::SECOND)
			throw ErrorType::BAD_PLAYER_NUMBER;

		this->playerNumber = playerNumber;

		srand(time(NULL));
	}

	void WhoseTurn::set(const PlayerNumber& playerNumber)
	{
		if (playerNumber != PlayerNumber::FIRST && playerNumber != PlayerNumber::SECOND)
			throw ErrorType::BAD_PLAYER_NUMBER;

		this->playerNumber = playerNumber;
	}

	void WhoseTurn::setTurnToNextPlayer()
	{
		playerNumber = (playerNumber == PlayerNumber::FIRST) ? PlayerNumber::SECOND : PlayerNumber::FIRST;
	}

	void WhoseTurn::setRandomly()
	{
		playerNumber = (rand() % 2) ? PlayerNumber::FIRST : PlayerNumber::SECOND;
	}
}

WhoseTurnTest.cpp

#include "CppUnitTest.h"
#include "WhoseTurn.h"

using namespace Microsoft::VisualStudio::CppUnitTestFramework;

namespace TicTacToeEngineTest
{		
	TEST_CLASS(WhoseTurnTest)
	{
	public:
		
		TEST_METHOD(WhoseTurnCheck)
		{
			ttt::WhoseTurn wt(ttt::WhoseTurn::PlayerNumber::FIRST);
			wt.setTurnToNextPlayer();
			Assert::IsTrue(wt.isSecondPlayerTurn());
			wt.setTurnToNextPlayer();
			Assert::IsTrue(wt.isFirstPlayerTurn());
		}

		TEST_METHOD(RandomWhoseTurn)
		{
			ttt::WhoseTurn wt(ttt::WhoseTurn::PlayerNumber::FIRST);		
			const int maxAmountOfTries = 1000000;
			for (int i = 0; i < maxAmountOfTries; i++)
			{
				wt.setRandomly();
				if (wt.isSecondPlayerTurn())
					break;
				else if (i + 1 == maxAmountOfTries)
					Assert::Fail();
			}
		}

		TEST_METHOD(BadPlayerNumber)
		{
			try {
				ttt::WhoseTurn wt(ttt::WhoseTurn::PlayerNumber(-1));
				Assert::Fail();
			}
			catch (ttt::WhoseTurn::ErrorType e) {
				;
			}
		}

	};
}

 

1 odpowiedź

+2 głosów
odpowiedź 4 listopada 2018 przez RafalS VIP (122,820 p.)
wybrane 4 listopada 2018 przez Huberti
 
Najlepsza

Fajnie, że napisałeś testy, ale kod jest kiepski.

  1. Nazwa klasy WhoseTurn brzmi dziwnie. Gdzie rzeczownik?
  2. setTurnToNextPlayer wewnatrz klasy ktorej jedyna odpowiedzialnoscia jest ustalanie kolejnosci graczy pokusilbym sie o krotsza nazwe
  3. Ta klasa jest troszke niepotrzebna. Ja rozumiem, że generalnie ładny kod ma dużo małych klas z jedną odpowiedzialnością, ale nie można przesadzać kosztem czytelności. Nie każdą flage trzeba opakowywać w 5 metod i testy jednostkowe. Całą funkcjonalność tej klasy można streścić do pojedyńczej flagi bool isFirstPlayerTurn i losowania pomiedzy 0 i 1. Oczywiscie o ile jestes 100% pewny ze zawsze bedzie tylko 2 zawodnikow, ale mówimy o grze w kółko i krzyżyk :D
  4. Czemu rzucasz enumem? Od tego są wyjątki. Nikt nie spodziewa sie, ze ktos mu rzuci enumem z biblioteki.
  5. Do losowosci w C++ jest biblioteka random, nie używaj staroci z C.
  6. Test losowosci jest troche dziwny. Sprawdzasz czy raz na milion padnie na ture pierwszego :D. Jesli juz chcesz testowac losowosc to moze zliczaj liczbe wystapien jednego i drugiego i sprawdz czy sa w miare rowne.
komentarz 4 listopada 2018 przez Huberti Gaduła (4,500 p.)

Dzięki za odpowiedź :D Rzeczywiście przekombinowałem z tą klasą. ;)

4. Czemu rzucasz enumem? Od tego są wyjątki. 

Nie do końca rozumiem. Jakie wyjątki Masz na myśli?

1
komentarz 4 listopada 2018 przez RafalS VIP (122,820 p.)
Rzucaj klase a nie enuma:

https://stackoverflow.com/questions/5048691/why-throw-a-class-over-an-enum

Najlepiej dziedziczącą pod std::exception:

https://isocpp.org/wiki/faq/exceptions#what-to-throw

bo wtedy bardzo łatwo jest złapać wszystko.
komentarz 7 listopada 2018 przez Huberti Gaduła (4,500 p.)

Dzięki za pomoc. Zerkniesz, czy coś można jeszcze zmienić?

Poprawiony kod:

#pragma once

#include <stdexcept>
#include <string>

namespace ttt {
	class ErrorType : public std::invalid_argument
	{
	public:
		ErrorType(const std::string& message) : std::invalid_argument(message) { }
	};

	class Turn
	{
	public:
		enum class Player { CIRCLE, SHARP };

		Turn(const Player& player = Player::CIRCLE);

		void set(const Player& player);

		void setNext();

		void setRandom();

		bool isCircleTurn() const { return player == Player::CIRCLE; };

		bool isSharpTurn() const { return player == Player::SHARP; };

	private:
		Player player;
	};
}
#include "WhoseTurn.h"
#include <random>

namespace ttt {
	Turn::Turn(const Player& player)
	{
		if (player != Player::CIRCLE && player != Player::SHARP)
			throw ErrorType("Invalid player");

		this->player = player;
	}

	void Turn::set(const Player& player)
	{
		if (player != Player::CIRCLE && player != Player::SHARP)
			throw ErrorType("Invalid player");

		this->player = player;
	}

	void Turn::setNext()
	{
		player = (player == Player::CIRCLE) ? Player::SHARP : Player::CIRCLE;
	}

	void Turn::setRandom()
	{
		std::random_device rd;
		std::mt19937 mt(rd());
		std::uniform_int_distribution<int> dist(0, 1);
		player = (dist(mt)) ? Player::CIRCLE : Player::SHARP;
	}
}
#include "CppUnitTest.h"
#include "WhoseTurn.h"

using namespace Microsoft::VisualStudio::CppUnitTestFramework;

namespace TicTacToeEngineTest
{		
	TEST_CLASS(TurnTest)
	{
	public:
		
		TEST_METHOD(TurnCheck)
		{
			ttt::Turn t(ttt::Turn::Player::CIRCLE);
			t.setNext();
			Assert::IsTrue(t.isSharpTurn());
			t.setNext();
			Assert::IsTrue(t.isCircleTurn());
		}

		TEST_METHOD(RandomTurn)
		{
			ttt::Turn t;		
			const long amountOfTries = 100000;
			long amountOfCircleTurn{}, amountOfSharpTurn{};
			for (long i = 0; i < amountOfTries; ++i)
			{
				t.setRandom();
				if (t.isCircleTurn())
					++amountOfCircleTurn;
				else if (t.isSharpTurn())
					++amountOfSharpTurn;
				else 
					Assert::Fail(L"Invalid player turn");
			}
			double circleTurnPercent = double(amountOfCircleTurn * 100) / amountOfTries,
				sharpTurnPercent = double(amountOfSharpTurn * 100) / amountOfTries;

			Assert::AreEqual(sharpTurnPercent, circleTurnPercent, 1.0);
		}

		TEST_METHOD(InvalidPlayer)
		{
			try {
				ttt::Turn t(ttt::Turn::Player(-1));
				Assert::Fail();
			}
			catch (ttt::ErrorType& e) {
				;
			}
		}

	};
}

 

Podobne pytania

0 głosów
2 odpowiedzi 191 wizyt
pytanie zadane 3 stycznia 2019 w C i C++ przez Huberti Gaduła (4,500 p.)
0 głosów
2 odpowiedzi 173 wizyt
pytanie zadane 2 października 2018 w C i C++ przez Huberti Gaduła (4,500 p.)
0 głosów
1 odpowiedź 149 wizyt
pytanie zadane 3 marca 2019 w C i C++ przez Huberti Gaduła (4,500 p.)

92,452 zapytań

141,261 odpowiedzi

319,074 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!

...