From: Warren Young
Date: March 15 2007 12:08am
Subject: Re: Why does ColData need two strings for the same data?
List-Archive: http://lists.mysql.com/plusplus/6428
Message-Id: <45F88E74.7040302@etr-usa.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Philippe Dirkse wrote:
> 
> I noticed the ColData template copying the 'c' string data
> passed to it during construction twice: once into itself (as a
> derivate from std::string) as the 'entire' buffer (so using the
> std::string(char*, int) constructor) but then also into a member
> std::string buf_ which only copies the contents till the first '\0'
> encountered. Is this really necessary? I can understand why you want
> to make a copy of the data, but twice?

Yes, you're right, this is inefficient and hard to justify.  I didn't 
write it, so my best guess at the reasoning behind it is that this is 
all defined in ColData_Tmpl, which is not *necessarily* instantiated 
with std::string or similar.  It reduces coupling between the template 
proper and the instantiations of it.

But, that's weak.  In all the years I've been maintaining and using this 
library, I've never seen anyone use ColData* in any way not prescribed 
by the library's current design.  For that matter, the typedef 
MutableColData is only used once within the library, and it's a 
questionable use to begin with.

For v3.0, I propose that we collapse this whole thing into a concrete 
class with no more functionality than is needed within the library.  We 
might even change the public inheritance from const std::string to a 
private inheritance, or even just a private data member.  We don't 
actually need an is-a relationship here.

For v2.3 (or thereabouts), all I think we can do is stop using the buf_ 
member, and keep everything in the parent class's data members.  We'll 
carry around a useless empty std::string instance, but we break the ABI 
if we go further.

> Another small suggestion/request: is it possible to add the function
> 'Row::raw_data(char* fieldName) to the Row class as a convenience
> function? Why support it for operator[] only? After searching the
> source I found that using
> raw_data(theRow.parent().field_num(fieldName)) also does the trick
> but give us some more (syntactic) sugar!!!

This feels a little odd, since raw_data shouldn't be all that widely 
used.  It's a low-level kind of thing, so why is sugar desirable here?

I'm guessing you're proposing this feature to cover the remaining 
weaknesses in the library's null character handling.  I'd rather fix 
that than add inappropriate sugar that we'll regret later.