-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Since the mail by cjhopman was eaten by the lovely gna.org spamfilters (twice!), I am now relaying it myself, so that all can read the stuff up easily. Find the mail at the end of my text or in the spam archives at gna.org[1]. It was also discussed a little in IRC today. If you ever notice that a mail of you does not appear at the list withhin a reasonable time (something like an hour, sometimes gna is not to fast), please check the spam archives and ping me. Cheers, Nils Kneuper aka Ivanovic
PS: No idea why the gna.org spam filters like to screw us this much... [1]https://mail.gna.org/public/spam/2009-07/msg05160.html [2]https://mail.gna.org/public/spam/ ********************************************* Hey- As Dave had discussed (http://dave.wesnoth.org/?p=9), I plan to change config::operator[] to config::set_attribute(). The main problem with the current interface is that it requires that we hold actual t_strings and that references to them are stable. This greatly restricts the config class internal representation. Another solution to this problem would be using a proxy class like in https://gna.org/patch/?1139. I believe that the proxy class solution is too complicated for the benefit it provides. That is, the two proposed solutions have interfaces: proxy_string operator[](const string& key); void set_attribute(const string& key, const t_string& val); The main reason that I prefer set_attribute is because it is precisely what we want, we want to set the attribute, we don't actually need a reference to the attribute or something that acts like one. The proxy_string version significantly complicates the interface of the config class as now we add a class that implements basically all of t_string to config's interface. Now, there are some reasons that people may prefer the operator[] version. Mordante said that cfg["asd"] = "qwe" is easier to type and read than cfg.set_attribute("asd", "qwe"). Silene said that cfg["asd""] = "qwe" was easier to read (and thus easier to get right). I am not convinced that it is easier to read. In fact, I feel that in some cases .set_attribute() is easier to read, in others [] is easier. Consider the two lines: (random->add_child("random"))["value"] = lexical_cast<std::string>(res); random->add_child("random").set_attribute("value", lexical_cast<std::string>(res)); Personally, I feel the second is clearer, but it is definitely a subjective judgment. Either way, set_attribute() is more explicit about what the caller wants and what is actually being done, especially compared to the proxy_string version (and not just the current interface). So, what do others think? If we change to set_attribute, I see two options for getting attribute values. We can keep the current operator[] const for getting values, but my feeling is that we should then use a get_attribute(). The reasons are that, config::operator[] const does not act quite like std::map::operator[], and I think get_attribute and set_attribute are more consistent than operator[] and set_attribute. Also, we could have, like get and set, cast_attribute(). That is, we would replace lexical_cast<int>(cfg["key"]) with cfg.cast_attribute<int>("key"). So, I am actually proposing that we get rid of both versions of config::operator[] and replace them with get_attribute() and set_attribute() and to add cast_attribute<T>() (and cast_attribute_default<T>()). - -Chris Hopman -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.11 (GNU/Linux) iEYEARECAAYFAkpWDUsACgkQfFda9thizwUuugCglJBwkHrOKkd9BvthF6ZViMM6 RJ4An3QT7+7Dx4c45yFXa9BPa8xIn0L5 =1snu -----END PGP SIGNATURE----- _______________________________________________ Wesnoth-dev mailing list [email protected] https://mail.gna.org/listinfo/wesnoth-dev
