Ocena kodu źródłowego

Question

Witam, proszę o ocenę poniższego kodu do gry tic tac toe.

GameBoard.h

#pragma once

#include <array>

namespace ttt {
	class GameBoard 
	{
	public:
		enum class Cell { EMPTY, CIRCLE, SHARP };

		GameBoard() 
			: gameBoard() {}

		GameBoard(const std::array<std::array<Cell, 3U>, 3U>& gameBoard) 
			: gameBoard(gameBoard) {}

		const Cell& operator()(unsigned row, unsigned column) const;

		Cell& operator()(unsigned row, unsigned column);

	private:
		std::array<std::array<Cell, 3U>, 3U> gameBoard;

		static const unsigned LAST_ROW = 2, LAST_COLUMN = 2;
	};
}

GameBoard.cpp

#include "GameBoard.h"
#include <stdexcept>

namespace ttt {
	const GameBoard::Cell& GameBoard::operator()(unsigned row, unsigned column) const
	{	
		if (row > LAST_ROW)
			throw std::invalid_argument("row out of bounds");
		else if (column > LAST_COLUMN)
			throw std::invalid_argument("column out of bounds");
		else
			return gameBoard[row][column];
	}

	GameBoard::Cell& GameBoard::operator()(unsigned row, unsigned column)
	{
		if (row > LAST_ROW)
			throw std::invalid_argument("row out of bounds");
		else if (column > LAST_COLUMN)
			throw std::invalid_argument("column out of bounds");
		else
			return gameBoard[row][column];
	}
}

Inicjalizacja w testach jednostkowych wygląda dosyć zawile. Czytając kod trzeba pomijać przedrostek ttt::GameBoard::Cell. Przykład:

ttt::GameBoard gb({
	ttt::GameBoard::Cell::EMPTY, ttt::GameBoard::Cell::EMPTY, ttt::GameBoard::Cell::EMPTY,
	ttt::GameBoard::Cell::EMPTY, ttt::GameBoard::Cell::EMPTY, ttt::GameBoard::Cell::EMPTY,
	ttt::GameBoard::Cell::SHARP, ttt::GameBoard::Cell::CIRCLE, ttt::GameBoard::Cell::SHARP
	});

Może lepiej jest zastosować enum zamiast enum class i wrzucić using ttt::GameBoard?
 

using ttt::GameBoard;

GameBoard gb({
	GameBoard::EMPTY, GameBoard::EMPTY, GameBoard::EMPTY,
	GameBoard::EMPTY, GameBoard::EMPTY, GameBoard::EMPTY,
	GameBoard::SHARP, GameBoard::CIRCLE, GameBoard::SHARP
	});

Ewentualnie przenieść enum class do osobnego pliku Cell.h

using ttt::Cell;
 
ttt::GameBoard gb({
    Cell::EMPTY, Cell::EMPTY, Cell::EMPTY,
    Cell::EMPTY, Cell::EMPTY, Cell::EMPTY,
    Cell::SHARP, Cell::CIRCLE, Cell::SHARP
    });

Czy może lepiej jest skorzystać ze zwykłej zmiennej typu char?

using ttt::GameBoard;

GameBoard gb({
	'n', 'n', 'n',
	'n', 'n', 'n',
	'X', 'O', 'X'
	});

 

Answer 1

Witam,

co do kodu to zgadzam się z Twoją sugestią

Może lepiej jest zastosować enum zamiast enum class i wrzucić using ttt::GameBoard?

Samo enum wystarczy. Tutaj dzięki klasie dane są zamknięte i nie musisz obawiać się o konflikt nazw. Co do przestrzeni nazw to radziłbym używać nazw bardziej precyzyjnych, które sugerują przeznaczenie. Polecam używanie przestrzeni nazw w pliku nagłówkowym*.h natomiast wykorzystanie jej poprzez using w pliku źrółowym *.cpp. Np. w pliku *h używaj std:: a w pliku cpp na początku dodaj using namespace std;.