Ocena kodu źródłowego

Question

Witam, proszę o ocenę klasy pod względem "czystego" kodu.

Klasa służy do zarządzania imieniem gracza.

#pragma once

#include <string>
#include <fstream>

namespace ttt {
	class NameOfOnePlayer
	{
	public:
		NameOfOnePlayer();

		void setFromInputStream();

		std::string get() const;

	private:
		enum Exception { CANNOT_OPEN, CORRUPT };
		static const std::string nameOfFile;

		std::string name;

		std::string getFromInputStream() const;

		bool hasNotProperSize(const std::string& name) const;

		bool isTooBig(const std::string &name) const;

		bool isTooSmall(const std::string &name) const;

		void setFromFile();

		void tryToSetFromFile();

		void setFrom(std::ifstream& file);

		void tryToSetFrom(std::ifstream& file);

		void saveToFile() const;

		void tryToSaveToFile() const;

		void saveTo(std::ofstream& file) const;
	};
}

#include "NameOfOnePlayer.h"
#include <iostream>

namespace ttt {
	const std::string NameOfOnePlayer::nameOfFile = "nameOfOnePlayer.txt";

	NameOfOnePlayer::NameOfOnePlayer()
	{		
		setFromFile();
	}

	void NameOfOnePlayer::setFromFile()
	{
		try {
			tryToSetFromFile();
		}
		catch (const Exception& e) {
			if (e == CANNOT_OPEN)
			{
				setFromInputStream();
				saveToFile();
			}				
		}
	}

	void NameOfOnePlayer::tryToSetFromFile()
	{
		std::ifstream file;
		file.open(nameOfFile);
		if (file.is_open())
		{
			setFrom(file);
			file.close();
		}
		else
		{
			file.close();
			throw CANNOT_OPEN;
		}
	}

	void NameOfOnePlayer::setFrom(std::ifstream& file)
	{
		try {
			tryToSetFrom(file);
		}
		catch (const Exception& e) {
			if (e == CORRUPT)
			{
				setFromInputStream();
				saveToFile();
			}				
		}
	}

	void NameOfOnePlayer::tryToSetFrom(std::ifstream& file)
	{
		if (!(file >> name && file.get()))
			throw CORRUPT;
	}

	void NameOfOnePlayer::setFromInputStream()
	{
		name = getFromInputStream();
	}

	std::string NameOfOnePlayer::getFromInputStream() const
	{
		std::string name;
		std::cout << "Podaj swoje imie: ";
		std::cin >> name;
		while (hasNotProperSize(name))
		{
			if (isTooBig(name))
			{
				std::cout << "Imie moze miec maksymalnie 12 znakow. Sprobuj jeszcze raz:";
				std::cin >> name;
			}
			else if (isTooSmall(name))
			{
				std::cout << "Imie musi miec przynajmniej 3 znaki. Sprobuj jeszcze raz:";
				std::cin >> name;
			}
		}
		return name;
	}

	bool NameOfOnePlayer::hasNotProperSize(const std::string& name) const
	{
		return isTooBig(name) || isTooSmall(name);
	}

	bool NameOfOnePlayer::isTooBig(const std::string &name) const
	{
		return name.size() > 12;
	}

	bool NameOfOnePlayer::isTooSmall(const std::string &name) const
	{
		return name.size() < 3;
	}

	void NameOfOnePlayer::saveToFile() const
	{
		try {
			tryToSaveToFile();
		}
		catch (const Exception& e) {
			if (e == CANNOT_OPEN)
				;//TODO log it to the file with communicates of errors
		}
		
	}

	void NameOfOnePlayer::tryToSaveToFile() const
	{
		std::ofstream file;
		file.open(nameOfFile);
		if (file.is_open())
		{
			saveTo(file);
			file.close();
		}		
		else
		{
			file.close();
			throw CANNOT_OPEN;
		}		
	}

	void NameOfOnePlayer::saveTo(std::ofstream& file) const
	{
		file << name << std::endl;
	}

	std::string NameOfOnePlayer::get() const
	{
		return name;
	}
}

Answer 1

Po pierwsze średnio znam c++ i  to będzie plus powiem ci jakie są odczucia osoby z innego języka.

Po pierwsze nie wiem czemu ten namesapce to ttt ale średnio czytelna nazwa ;)
jak już try zapisujesz tak

try {
    //
} catch(const Exception& e) {
     //
}

to if też bym tak zapisywał

if (true) {
    //
} else {
    //
}

nie umieszczał bym cout i cin w takiej kalsie, dla mnie to trochę niezgodne z OOP klasa która stertuje I/O dałbym osobno. Tak samo oddzieliłbym obsługę pliku do innej klasy. Wmyśl także zasad SOLID. 

Po za tym jest naprawdę dobrze, rozwijaj się powodzenia :D 

Answer 2

Linia 58: jeśli to get w warunku jest po to, żeby usunąć znak końca linii, to lepiej użyj ignore.