Merge pull request #226 from gfgtdf/config_comparision

add operators ==,!= (attribute_value, std::string)

previously comparisions like c["a"] = "b" were evaluated by 
inline bool operator==(const std::string &a, const t_string &b) 
by casting atribute_value to std::string, and const char * to t_string

we fix this by adding explicit equality operator for these cases.

Not creating t_string also results in significant preformance improvements in some cases.

src/config.cpp

@@ -31,6 +31,7 @@
 #include <deque>
 #include <istream>
 
+#include <boost/bind.hpp>
 #include <boost/foreach.hpp>
 #include <boost/variant/apply_visitor.hpp>
 #include <boost/variant/get.hpp>
@@ -390,6 +391,16 @@ bool config::attribute_value::operator==(const config::attribute_value &other) c
 	return boost::apply_visitor(equality_visitor(), value_, other.value_);
 }
 
+/**
+ * Checks for equality of the attribute values when viewed as strings.
+ * Exception: Boolean synonyms can be equal ("yes" == "true").
+ * Note: Blanks have no string representation, so do not equal "" (an empty string).
+ */
+bool config::attribute_value::equals(const std::string &str) const
+{
+	return boost::apply_visitor(boost::bind( equality_visitor(), _1, boost::cref(str) ), value_);
+}
+
 std::ostream &operator<<(std::ostream &os, const config::attribute_value &v)
 {
 	// Simple implementation, but defined out-of-line because of the templating

src/config.hpp

@@ -42,6 +42,10 @@
 #include <boost/variant/apply_visitor.hpp>
 #include <boost/variant/variant.hpp>
 
+#include <boost/utility/enable_if.hpp>
+#include <boost/type_traits/is_same.hpp>
+#include <boost/type_traits/add_const.hpp>
+
 #include "exceptions.hpp"
 #include "tstring.hpp"
 
@@ -281,6 +285,32 @@ class config
 		bool operator!=(const attribute_value &other) const
 		{ return !operator==(other); }
 
+		// returns always false if the underlying type is no string.
+		bool equals(const std::string& str) const;
+		// These function prevent t_string creation in case of c["a"] == "b" comparisons.
+		// The templates are needed to prevent using these function in case of c["a"] == 0 comparisons.
+		template<typename T>
+		typename boost::enable_if<boost::is_same<const std::string, typename boost::add_const<T>::type>, bool>::type
+		friend operator==(const attribute_value &val, const T &str)
+		{ return val.equals(str); }
+
+		template<typename T>
+		typename boost::enable_if<boost::is_same<const char*, T>, bool>::type
+		friend operator==(const attribute_value &val, T str)
+		{ return val.equals(std::string(str)); }
+
+		template<typename T>
+		bool friend operator==(const T &str, const attribute_value &val)
+		{ return val == str; }
+
+		template<typename T>
+		bool friend operator!=(const attribute_value &val, const T &str)
+		{ return !(val == str); }
+
+		template<typename T>
+		bool friend operator!=(const T &str, const attribute_value &val)
+		{ return !(val == str); }
+
 		// Streaming:
 		friend std::ostream& operator<<(std::ostream &os, const attribute_value &v);
 

src/generators/map_create.cpp

@@ -22,6 +22,7 @@
 #include "scoped_resource.hpp"
 #include "serialization/string_utils.hpp"
 
+#include <cassert>
 
 static lg::log_domain log_config("config");
 #define ERR_CF LOG_STREAM(err, log_config)
@@ -46,6 +47,7 @@ std::string random_generate_map(const std::string& parms, const config &cfg)
 	//the first token is the name of the generator, tokens after
 	//that are arguments to the generator
 	std::vector<std::string> parameters = utils::split(parms, ' ');
+	assert(!parameters.empty()); //we use parameters.front() in the next line.
 	util::scoped_ptr<map_generator> generator(create_map_generator(parameters.front(),cfg));
 	if(generator == NULL) {
 		ERR_CF << "could not find map generator '" << parameters.front() << "'" << std::endl;

src/help.cpp

@@ -958,7 +958,7 @@ void parse_config_internal(const config *help_cfg, const config *section_cfg,
 		} else if ((*section_cfg)["sort_topics"] == "generated") {
 		  sort_topics = false;
 		  sort_generated = true;
-		} else if ((*section_cfg)["sort_topics"] != "") {
+		} else if (!(*section_cfg)["sort_topics"].empty()) {
 		  std::stringstream ss;
 		  ss << "Invalid sort option: '" << (*section_cfg)["sort_topics"] << "'";
 		  throw parse_error(ss.str());

src/multiplayer_wait.cpp

@@ -464,7 +464,7 @@ void wait::process_network_data(const config& data, const network::connection so
 {
 	ui::process_network_data(data, sock);
 
-	if(data["message"] != "") {
+	if(!data["message"].empty()) {
 		gui2::show_transient_message(disp().video()
 				, _("Response")
 				, data["message"]);

src/saved_game.cpp

@@ -315,7 +315,7 @@ void saved_game::expand_random_scenario()
 	if(this->starting_pos_type_ == STARTINGPOS_SCENARIO)
 	{
 		// If the entire scenario should be randomly generated
-		if(starting_pos_["scenario_generation"] != "") 
+		if(!starting_pos_["scenario_generation"].empty()) 
 		{
 			LOG_NG << "randomly generating scenario...\n";
 			const cursor::setter cursor_setter(cursor::WAIT);
@@ -331,12 +331,12 @@ void saved_game::expand_random_scenario()
 			update_label();
 		}
 		//it looks like we support a map= where map=filename equals more or less map_data={filename}
-		if(starting_pos_["map_data"].empty() && starting_pos_["map"] != "") {
+		if(starting_pos_["map_data"].empty() && !starting_pos_["map"].empty()) {
 			starting_pos_["map_data"] = read_map(starting_pos_["map"]);
 		}
 		// If the map should be randomly generated
 		// We dont want that we accidently to this twice so we check for starting_pos_["map_data"].empty()
-		if(starting_pos_["map_data"].empty() && starting_pos_["map_generation"] != "") {
+		if(starting_pos_["map_data"].empty() && !starting_pos_["map_generation"].empty()) {
 			LOG_NG << "randomly generating map...\n";
 			const cursor::setter cursor_setter(cursor::WAIT);
 

src/tests/test_config.cpp

@@ -24,6 +24,7 @@ BOOST_AUTO_TEST_SUITE ( test_config )
 BOOST_AUTO_TEST_CASE ( test_config_attribute_value )
 {
 	config c;
+	const config& cc = c;
 	int x_int;
 	std::string x_str;
 	long long x_sll;
@@ -146,6 +147,29 @@ BOOST_AUTO_TEST_CASE ( test_config_attribute_value )
 		BOOST_CHECK_EQUAL(x_sll, 123456789123ll);
 	x_str = c["x"].str();
 		BOOST_CHECK_EQUAL(x_str, "123456789123");
+
+		// blank != "" test.
+	c = config();
+	BOOST_CHECK(cc["x"] != "");
+	BOOST_CHECK(cc["x"].empty());
+	BOOST_CHECK(cc["x"].blank());
+
+	BOOST_CHECK(c["x"] != "");
+	BOOST_CHECK(c["x"].empty());
+	BOOST_CHECK(c["x"].blank());
+
+	BOOST_CHECK_EQUAL(cc["x"], c["x"]);
+
+	c["x"] = "";
+	BOOST_CHECK(cc["x"] == "");
+	BOOST_CHECK(cc["x"].empty());
+	BOOST_CHECK(!cc["x"].blank());
+
+	BOOST_CHECK(c["x"] == "");
+	BOOST_CHECK(c["x"].empty());
+	BOOST_CHECK(!c["x"].blank());
+
+	BOOST_CHECK_EQUAL(cc["x"], c["x"]);
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/unit.cpp

@@ -1811,7 +1811,7 @@ int unit::defense_modifier(const t_translation::t_terrain & terrain) const
 
 bool unit::resistance_filter_matches(const config& cfg, bool attacker, const std::string& damage_name, int res) const
 {
-	if(!(cfg["active_on"]=="" || (attacker && cfg["active_on"]=="offense") || (!attacker && cfg["active_on"]=="defense"))) {
+	if(!(cfg["active_on"].empty() || (attacker && cfg["active_on"]=="offense") || (!attacker && cfg["active_on"]=="defense"))) {
 		return false;
 	}
 	const std::string& apply_to = cfg["apply_to"];

src/unit_types.cpp

@@ -1110,7 +1110,7 @@ int unit_type::resistance_against(const std::string& damage_name, bool attacker)
 
 bool unit_type::resistance_filter_matches(const config& cfg, bool attacker, const std::string& damage_name, int res) const
 {
-	if(!(cfg["active_on"]=="" || (attacker && cfg["active_on"]=="offense") || (!attacker && cfg["active_on"]=="defense"))) {
+	if(!(cfg["active_on"].empty() || (attacker && cfg["active_on"]=="offense") || (!attacker && cfg["active_on"]=="defense"))) {
 		return false;
 	}
 	const std::string& apply_to = cfg["apply_to"];