Refactor PlayerAttributes code

src/PlayerAttributes.cpp

@@ -2,12 +2,129 @@
 #include <sstream>
 #include <stdexcept>
 #include <dirent.h>
-#include <unistd.h> // for usleep
+#include <unistd.h> // usleep
-#include <sqlite3.h>
 
 #include "config.h"
 #include "PlayerAttributes.h"
 #include "util.h"
+#include "db-sqlite3.h" // SQLite3Base
+
+namespace {
+	bool parse_pos(std::string position, Player &dst)
+	{
+		if (position.empty())
+			return false;
+		if (position.front() == '(' && position.back() == ')')
+			position = position.substr(1, position.size() - 2);
+		std::istringstream iss(position);
+		if (!(iss >> dst.x))
+			return false;
+		if (iss.get() != ',')
+			return false;
+		if (!(iss >> dst.y))
+			return false;
+		if (iss.get() != ',')
+			return false;
+		if (!(iss >> dst.z))
+			return false;
+		return iss.eof();
+	}
+
+	// Helper classes per backend
+
+	class FilesReader {
+		std::string path;
+		DIR *dir = nullptr;
+	public:
+		FilesReader(const std::string &path) : path(path) {
+			dir = opendir(path.c_str());
+		}
+		~FilesReader() {
+			if (dir)
+				closedir(dir);
+		}
+
+		void read(PlayerAttributes::Players &dest);
+	};
+
+	class SQLiteReader : SQLite3Base {
+		sqlite3_stmt *stmt_get_player_pos = NULL;
+	public:
+		SQLiteReader(const std::string &database) {
+			openDatabase(database.c_str());
+		}
+		~SQLiteReader() {
+			sqlite3_finalize(stmt_get_player_pos);
+		}
+
+		void read(PlayerAttributes::Players &dest);
+	};
+}
+
+void FilesReader::read(PlayerAttributes::Players &dest)
+{
+	if (!dir)
+		return;
+
+	struct dirent *ent;
+	std::string name, position;
+	while ((ent = readdir(dir)) != NULL) {
+		if (ent->d_name[0] == '.')
+			continue;
+
+		std::ifstream in(path + PATH_SEPARATOR + ent->d_name);
+		if (!in.good())
+			continue;
+
+		name = read_setting("name", in);
+		position = read_setting("position", in);
+
+		Player player;
+		player.name = name;
+		if (!parse_pos(position, player)) {
+			std::cerr << "Failed to parse position " << position << std::endl;
+			continue;
+		}
+
+		player.x /= 10.0f;
+		player.y /= 10.0f;
+		player.z /= 10.0f;
+
+		dest.push_back(std::move(player));
+	}
+}
+
+#define SQLRES(r, good) check_result(r, good)
+#define SQLOK(r) SQLRES(r, SQLITE_OK)
+
+void SQLiteReader::read(PlayerAttributes::Players &dest)
+{
+	SQLOK(prepare(stmt_get_player_pos,
+			"SELECT name, posX, posY, posZ FROM player"));
+
+	int result;
+	while ((result = sqlite3_step(stmt_get_player_pos)) != SQLITE_DONE) {
+		if (result == SQLITE_BUSY) { // Wait some time and try again
+			usleep(10000);
+		} else if (result != SQLITE_ROW) {
+			throw std::runtime_error(sqlite3_errmsg(db));
+		}
+
+		Player player;
+		player.name = read_str(stmt_get_player_pos, 0);
+		player.x = sqlite3_column_double(stmt_get_player_pos, 1);
+		player.y = sqlite3_column_double(stmt_get_player_pos, 2);
+		player.z = sqlite3_column_double(stmt_get_player_pos, 3);
+
+		player.x /= 10.0f;
+		player.y /= 10.0f;
+		player.z /= 10.0f;
+
+		dest.push_back(std::move(player));
+	}
+}
+
+/**********/
 
 PlayerAttributes::PlayerAttributes(const std::string &worldDir)
 {
@@ -18,107 +135,13 @@ PlayerAttributes::PlayerAttributes(const std::string &worldDir)
 	ifs.close();
 
 	if (backend == "files")
-		readFiles(worldDir + "players");
+		FilesReader(worldDir + "players").read(m_players);
 	else if (backend == "sqlite3")
-		readSqlite(worldDir + "players.sqlite");
+		SQLiteReader(worldDir + "players.sqlite").read(m_players);
 	else
 		throw std::runtime_error(std::string("Unknown player backend: ") + backend);
 }
 
-void PlayerAttributes::readFiles(const std::string &playersPath)
-{
-	DIR *dir;
-	dir = opendir (playersPath.c_str());
-	if (!dir)
-		return;
-
-	struct dirent *ent;
-	while ((ent = readdir (dir)) != NULL) {
-		if (ent->d_name[0] == '.')
-			continue;
-
-		std::ifstream in(playersPath + PATH_SEPARATOR + ent->d_name);
-		if (!in.good())
-			continue;
-
-		std::string name, position;
-		name = read_setting("name", in);
-		in.seekg(0);
-		position = read_setting("position", in);
-
-		Player player;
-		std::istringstream iss(position);
-		char tmp;
-		iss >> tmp; // '('
-		iss >> player.x;
-		iss >> tmp; // ','
-		iss >> player.y;
-		iss >> tmp; // ','
-		iss >> player.z;
-		iss >> tmp; // ')'
-		if (tmp != ')')
-			continue;
-		player.name = name;
-
-		player.x /= 10.0f;
-		player.y /= 10.0f;
-		player.z /= 10.0f;
-
-		m_players.push_back(player);
-	}
-
-	closedir(dir);
-}
-
-/**********/
-
-#define SQLRES(f, good) \
-	result = (sqlite3_##f); \
-	if (result != good) { \
-		throw std::runtime_error(sqlite3_errmsg(db));\
-	}
-#define SQLOK(f) SQLRES(f, SQLITE_OK)
-
-void PlayerAttributes::readSqlite(const std::string &db_name)
-{
-	int result;
-	sqlite3 *db;
-	sqlite3_stmt *stmt_get_player_pos;
-
-	SQLOK(open_v2(db_name.c_str(), &db, SQLITE_OPEN_READONLY |
-			SQLITE_OPEN_PRIVATECACHE, 0))
-
-	SQLOK(prepare_v2(db,
-			"SELECT name, posX, posY, posZ FROM player",
-		-1, &stmt_get_player_pos, NULL))
-
-	while ((result = sqlite3_step(stmt_get_player_pos)) != SQLITE_DONE) {
-		if (result == SQLITE_BUSY) { // Wait some time and try again
-			usleep(10000);
-		} else if (result != SQLITE_ROW) {
-			throw std::runtime_error(sqlite3_errmsg(db));
-		}
-
-		Player player;
-		const unsigned char *name_ = sqlite3_column_text(stmt_get_player_pos, 0);
-		player.name = reinterpret_cast<const char*>(name_);
-		player.x = sqlite3_column_double(stmt_get_player_pos, 1);
-		player.y = sqlite3_column_double(stmt_get_player_pos, 2);
-		player.z = sqlite3_column_double(stmt_get_player_pos, 3);
-
-		player.x /= 10.0f;
-		player.y /= 10.0f;
-		player.z /= 10.0f;
-
-		m_players.push_back(player);
-	}
-
-	sqlite3_finalize(stmt_get_player_pos);
-	sqlite3_close(db);
-}
-
-/**********/
-
 PlayerAttributes::Players::const_iterator PlayerAttributes::begin() const
 {
 	return m_players.cbegin();

src/PlayerAttributes.h

@@ -19,8 +19,5 @@ public:
 	Players::const_iterator end() const;
 
 private:
-	void readFiles(const std::string &playersPath);
-	void readSqlite(const std::string &db_name);
-
 	Players m_players;
 };

src/db-sqlite3.h

@@ -24,6 +24,14 @@ protected:
 		return sqlite3_prepare_v2(db, sql, -1, &stmt, NULL);
 	}
 
+	// read string from statement
+	static inline std::string read_str(sqlite3_stmt *stmt, int iCol)
+	{
+		auto *data = reinterpret_cast<const char*>(
+			sqlite3_column_text(stmt, iCol));
+		return std::string(data);
+	}
+
 	// read blob from statement
 	static inline ustring read_blob(sqlite3_stmt *stmt, int iCol)
 	{

src/util.cpp

@@ -1,5 +1,5 @@
 #include <stdexcept>
-#include <sstream>
+#include <iostream>
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -21,42 +21,45 @@ static std::string trim(const std::string &s)
 	return s.substr(front, back - front + 1);
 }
 
-std::string read_setting(const std::string &name, std::istream &is)
+static bool read_setting(const std::string &name, std::istream &is, std::string &out)
 {
 	char linebuf[512];
 	is.seekg(0);
 	while (is.good()) {
 		is.getline(linebuf, sizeof(linebuf));
-
-		for (char *p = linebuf; *p; p++) {
-			if(*p != '#')
-				continue;
-			*p = '\0'; // Cut off at the first #
-			break;
-		}
 		std::string line(linebuf);
 
-		auto pos = line.find('=');
+		auto pos = line.find('#');
+		if (pos != std::string::npos)
+			line.erase(pos); // remove comments
+
+		pos = line.find('=');
 		if (pos == std::string::npos)
 			continue;
 		auto key = trim(line.substr(0, pos));
 		if (key != name)
 			continue;
-		return trim(line.substr(pos+1));
+		out = trim(line.substr(pos+1));
+		return true;
 	}
-	std::ostringstream oss;
+	return false;
-	oss << "Setting '" << name << "' not found";
+}
-	throw std::runtime_error(oss.str());
+
+std::string read_setting(const std::string &name, std::istream &is)
+{
+	std::string ret;
+	if (!read_setting(name, is, ret))
+		throw std::runtime_error(std::string("Setting not found: ") + name);
+	return ret;
 }
 
 std::string read_setting_default(const std::string &name, std::istream &is,
 	const std::string &def)
 {
-	try {
+	std::string ret;
-		return read_setting(name, is);
+	if (!read_setting(name, is, ret))
-	} catch(const std::runtime_error &e) {
 		return def;
-	}
+	return ret;
 }
 
 bool file_exists(const char *path)