Use dedicated error enum for parsing failures

Source/data/file.cpp

@@ -47,24 +47,24 @@ void DataFile::reportFatalError(Error code, std::string_view fileName)
 	}
 }
 
-void DataFile::reportFatalFieldError(std::errc code, std::string_view fileName, std::string_view fieldName, const DataFileField &field)
+void DataFile::reportFatalFieldError(DataFileField::Error code, std::string_view fileName, std::string_view fieldName, const DataFileField &field)
 {
 	switch (code) {
-	case std::errc::invalid_argument:
+	case DataFileField::Error::NotANumber:
 		app_fatal(fmt::format(fmt::runtime(_(
 		                          /* TRANSLATORS: Error message when parsing a data file and a text value is encountered when a number is expected. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
 		                          "Non-numeric value {0} for {1} in {2} at row {3} and column {4}")),
 		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
-	case std::errc::result_out_of_range:
+	case DataFileField::Error::OutOfRange:
 		app_fatal(fmt::format(fmt::runtime(_(
 		                          /* TRANSLATORS: Error message when parsing a data file and we find a number larger than expected. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
 		                          "Out of range value {0} for {1} in {2} at row {3} and column {4}")),
 		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
-	default:
+	case DataFileField::Error::InvalidValue:
 		app_fatal(fmt::format(fmt::runtime(_(
-		                          /* TRANSLATORS: Fallback error message when an error occurs while parsing a data file and we can't determine the cause. Arguments are {file name}, {row/record number}, {column/field number} */
-		                          "Unexpected error while reading {0} at row {1} and column {2}")),
-		    fileName, field.row(), field.column()));
+		                          /* TRANSLATORS: Error message when we find an unrecognised value in a key column. Arguments are {found value}, {column heading}, {file name}, {row/record number}, {column/field number} */
+		                          "Invalid value {0} for {1} in {2} at row {3} and column {4}")),
+		    field.currentValue(), fieldName, fileName, field.row(), field.column()));
 	}
 }
 

Source/data/file.hpp

@@ -75,7 +75,7 @@ class DataFile {
 	static tl::expected<DataFile, Error> load(std::string_view path);
 
 	static void reportFatalError(Error code, std::string_view fileName);
-	static void reportFatalFieldError(std::errc code, std::string_view fileName, std::string_view fieldName, const DataFileField &field);
+	static void reportFatalFieldError(DataFileField::Error code, std::string_view fileName, std::string_view fieldName, const DataFileField &field);
 
 	void resetHeader()
 	{

Source/data/iterators.hpp

@@ -16,6 +16,26 @@ class DataFileField {
 	unsigned column_;
 
 public:
+	enum class Error {
+		NotANumber,
+		OutOfRange,
+		InvalidValue
+	};
+
+	static tl::expected<void, Error> mapError(std::errc ec)
+	{
+		switch (ec) {
+		case std::errc():
+			return {};
+		case std::errc::result_out_of_range:
+			return tl::unexpected { Error::OutOfRange };
+		case std::errc::invalid_argument:
+			return tl::unexpected { Error::NotANumber };
+		default:
+			return tl::unexpected { Error::InvalidValue };
+		}
+	}
+
 	DataFileField(GetFieldResult *state, const char *end, unsigned row, unsigned column)
 	    : state_(state)
 	    , end_(end)
@@ -56,10 +76,10 @@ class DataFileField {
 	 * use with operator* or repeated calls to parseInt (even with different types).
 	 * @tparam T an Integral type supported by std::from_chars
 	 * @param destination value to store the result of successful parsing
-	 * @return the error code from std::from_chars
+	 * @return an error code corresponding to the from_chars result if parsing failed
 	 */
 	template <typename T>
-	[[nodiscard]] std::errc parseInt(T &destination)
+	[[nodiscard]] tl::expected<void, Error> parseInt(T &destination)
 	{
 		std::from_chars_result result {};
 		if (state_->status == GetFieldResult::Status::ReadyToRead) {
@@ -74,18 +94,15 @@ class DataFileField {
 		} else {
 			result = std::from_chars(state_->value.data(), end_, destination);
 		}
-		return result.ec;
+
+		return mapError(result.ec);
 	}
 
 	template <typename T>
-	[[nodiscard]] tl::expected<T, std::errc> asInt()
+	[[nodiscard]] tl::expected<T, Error> asInt()
 	{
 		T value = 0;
-		auto parseIntResult = parseInt(value);
-		if (parseIntResult == std::errc()) {
-			return value;
-		}
-		return tl::unexpected { parseIntResult };
+		return parseInt(value).map([value]() { return value; });
 	}
 
 	/**

Source/playerdat.cpp

@@ -120,20 +120,20 @@ void ReloadExperienceData()
 			case ExperienceColumn::Level: {
 				auto parseIntResult = field.parseInt(level);
 
-				if (parseIntResult != std::errc()) {
+				if (!parseIntResult.has_value()) {
 					if (*field == "MaxLevel") {
 						skipRecord = true;
 					} else {
-						DataFile::reportFatalFieldError(parseIntResult, filename, "Level", field);
+						DataFile::reportFatalFieldError(parseIntResult.error(), filename, "Level", field);
 					}
 				}
 			} break;
 
 			case ExperienceColumn::Experience: {
 				auto parseIntResult = field.parseInt(experience);
 
-				if (parseIntResult != std::errc()) {
-					DataFile::reportFatalFieldError(parseIntResult, filename, "Experience", field);
+				if (!parseIntResult.has_value()) {
+					DataFile::reportFatalFieldError(parseIntResult.error(), filename, "Experience", field);
 				}
 			} break;
 

test/data_file_test.cpp

@@ -214,7 +214,11 @@ TEST(DataFileTest, ParseInt)
 		DataFileField field = *fieldIt;
 		uint8_t shortVal = 5;
 		auto parseIntResult = field.parseInt(shortVal);
-		EXPECT_EQ(parseIntResult, std::errc::invalid_argument) << "Strings are not uint8_t values";
+		if (parseIntResult.has_value()) {
+			ADD_FAILURE() << "Parsing a string as an int should not succeed";
+		} else {
+			EXPECT_EQ(parseIntResult.error(), DataFileField::Error::NotANumber) << "Strings are not uint8_t values";
+		}
 		EXPECT_EQ(shortVal, 5) << "Value is not modified when parsing as uint8_t fails due to non-numeric fields";
 		EXPECT_EQ(*field, "Sample") << "Should be able to access the field value as a string after failure";
 		++fieldIt;
@@ -225,7 +229,7 @@ TEST(DataFileTest, ParseInt)
 		field = *fieldIt;
 		shortVal = 5;
 		parseIntResult = field.parseInt(shortVal);
-		EXPECT_EQ(parseIntResult, std::errc()) << "Expected " << field << "to fit into a uint8_t variable";
+		EXPECT_TRUE(parseIntResult.has_value()) << "Expected " << field << " to fit into a uint8_t variable";
 		EXPECT_EQ(shortVal, 145) << "Parsing should give the expected base 10 value";
 		EXPECT_EQ(*field, "145") << "Should be able to access the field value as a string even after parsing as an int";
 		++fieldIt;
@@ -235,11 +239,15 @@ TEST(DataFileTest, ParseInt)
 		// Third field is a number too large for a uint8_t but that fits into an int value
 		field = *fieldIt;
 		parseIntResult = field.parseInt(shortVal);
-		EXPECT_EQ(parseIntResult, std::errc::result_out_of_range) << "A value too large to fit into a uint8_t variable should report an error";
+		if (parseIntResult.has_value()) {
+			ADD_FAILURE() << "Parsing an int into a short variable should not succeed";
+		} else {
+			EXPECT_EQ(parseIntResult.error(), DataFileField::Error::OutOfRange) << "A value too large to fit into a uint8_t variable should report an error";
+		}
 		EXPECT_EQ(shortVal, 145) << "Value is not modified when parsing as uint8_t fails due to out of range value";
 		int longVal = 42;
 		parseIntResult = field.parseInt(longVal);
-		EXPECT_EQ(parseIntResult, std::errc()) << "Expected " << field << "to fit into an int variable";
+		EXPECT_TRUE(parseIntResult.has_value()) << "Expected " << field << " to fit into an int variable";
 		EXPECT_EQ(longVal, 70322) << "Value is expected to be parsed into a larger type after an out of range failure";
 		EXPECT_EQ(*field, "70322") << "Should be able to access the field value as a string after parsing as an int";
 		++fieldIt;
@@ -249,7 +257,7 @@ TEST(DataFileTest, ParseInt)
 		// Fourth field is not an integer, but a value that starts with one or more digits that fit into an uint8_t value
 		field = *fieldIt;
 		parseIntResult = field.parseInt(shortVal);
-		EXPECT_EQ(parseIntResult, std::errc()) << "Expected " << field << "to fit into a uint8_t variable (even though it's not really an int)";
+		EXPECT_TRUE(parseIntResult.has_value()) << "Expected " << field << " to fit into a uint8_t variable (even though it's not really an int)";
 		EXPECT_EQ(shortVal, 6) << "Value is loaded as expected until the first non-digit character";
 		EXPECT_EQ(*field, "6.34") << "Should be able to access the field value as a string after failure";
 	}