Difference between revisions of "SummerOfCodeProposal res"
(→Next (technically, first) Steps) |
(→Current status) |
||
Line 8: | Line 8: | ||
* Upon reading the blog post linked in the idea proposal my first thought was to use a proxy class for operator[]. Below a patch that implements it. | * Upon reading the blog post linked in the idea proposal my first thought was to use a proxy class for operator[]. Below a patch that implements it. | ||
+ | * I submitted a proposal via the Google SoC app. It contains some other information relevant for mentors/admins. | ||
== Proxy class patch == | == Proxy class patch == | ||
Line 13: | Line 14: | ||
This builds and, I believe, works, but has a number of not-so-nice issues: | This builds and, I believe, works, but has a number of not-so-nice issues: | ||
* Some places store return values in <tt>t_string&</tt>. The idea is to change these to <tt>config::proxy_string</tt>, as that type should ideally behave like <tt>t_string&</tt>. | * Some places store return values in <tt>t_string&</tt>. The idea is to change these to <tt>config::proxy_string</tt>, as that type should ideally behave like <tt>t_string&</tt>. | ||
− | * <tt>config::proxy_string</tt> can cast to both <tt>std::string</tt> and <tt>t_string</tt>. | + | * <s><tt>config::proxy_string</tt> can cast to both <tt>std::string</tt> and <tt>t_string</tt>. |
The first is necessary as <tt>t_string</tt> can cast to it as well; the second is necessary to | The first is necessary as <tt>t_string</tt> can cast to it as well; the second is necessary to | ||
support use of a <tt>config::proxy_string</tt> where a <tt>const t_string&</tt> is expected. | support use of a <tt>config::proxy_string</tt> where a <tt>const t_string&</tt> is expected. | ||
− | Unfortunately, this introduces an ambiguity in cases where a <tt>config::proxy_string</tt> is passed to methods that can take either of <tt>std::string</tt> or <tt>t_string</tt>. The ambiguity was solved in some places by using <tt>.str()</tt>. It would also go away when <tt>proxy_string operator[]</tt> is removed. | + | Unfortunately, this introduces an ambiguity in cases where a <tt>config::proxy_string</tt> is passed to methods that can take either of <tt>std::string</tt> or <tt>t_string</tt>. The ambiguity was solved in some places by using <tt>.str()</tt>. It would also go away when <tt>proxy_string operator[]</tt> is removed.</s> Resolved differently, see comments on GNA. |
* the <tt>variables_set</tt> class has a member (<tt>get_variable_const()</tt>) that returns a <tt>t_string&</tt>. <tt>game_state</tt> descends from this class; it's <tt>get_variable_const()</tt> implementation supposedly returns a value from a <tt>config</tt>. To ensure the ref is valid, the value as a <tt>t_string</tt> is stored in a map. This is clearly not ideal. | * the <tt>variables_set</tt> class has a member (<tt>get_variable_const()</tt>) that returns a <tt>t_string&</tt>. <tt>game_state</tt> descends from this class; it's <tt>get_variable_const()</tt> implementation supposedly returns a value from a <tt>config</tt>. To ensure the ref is valid, the value as a <tt>t_string</tt> is stored in a map. This is clearly not ideal. | ||
Revision as of 13:56, 27 March 2009
A bit about me
My name is Frank Richter, currently studying Computer Sciences at the University of Jena (that's in Germany).
As far as C++ is concerned, I program in it since at least seven years now. In my spare time I also develop for Crystal Space, an open source 3D engine. While I did a lot in the topics of realtime 3D, OpenGL, shaders and such, I also did (and do) 'low-level' things like cross-platform services and, last, but in this context here now certainly not least, matters of memory optimization.
Current status
- Upon reading the blog post linked in the idea proposal my first thought was to use a proxy class for operator[]. Below a patch that implements it.
- I submitted a proposal via the Google SoC app. It contains some other information relevant for mentors/admins.
Proxy class patch
This builds and, I believe, works, but has a number of not-so-nice issues:
- Some places store return values in t_string&. The idea is to change these to config::proxy_string, as that type should ideally behave like t_string&.
config::proxy_string can cast to both std::string and t_string.
The first is necessary as t_string can cast to it as well; the second is necessary to
support use of a config::proxy_string where a const t_string& is expected.
Unfortunately, this introduces an ambiguity in cases where a config::proxy_string is passed to methods that can take either of std::string or t_string. The ambiguity was solved in some places by using .str(). It would also go away when proxy_string operator[] is removed. Resolved differently, see comments on GNA.
- the variables_set class has a member (get_variable_const()) that returns a t_string&. game_state descends from this class; it's get_variable_const() implementation supposedly returns a value from a config. To ensure the ref is valid, the value as a t_string is stored in a map. This is clearly not ideal.
The patch was submitted to the patch tracker: https://gna.org/patch/index.php?1139
Next (technically, first) Steps
- Get rid of t_string& operator[] - could be a proxy class, as above, or like in the initial blog post, a setter function. A way in-between could be to have the proxy class for convenient migration, but mark is a deprecated; instead, promote the usage of the setter. Cases where the proxy class actually introduces ugliness should be fixed manually.
- Assuming that only a limited set of keys is used all keys can be stored in a global "add-only" pool. Individual config instances would only the pointers to the global keys.
- For values consider changing t_string to be backed by a string that is "copy on write", so a single copy is used unless the string is manipulated. silene suggested to store short, untranslatable values inside t_string directly to use the "16 or 24 bytes" it takes up anyway.