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.
