lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] Empty xml child text nodes [Was: product editor patch]


From: Greg Chicares
Subject: [lmi] Empty xml child text nodes [Was: product editor patch]
Date: Sun, 30 Mar 2008 17:17:32 +0000
User-agent: Thunderbird 2.0.0.12 (Windows/20080213)

On 2008-03-28 13:38Z, Vaclav Slavik wrote:
> Greg Chicares wrote:
[...]
>> (D) inputillus_xml_io.cpp, ledger_xml_io.cpp, xml_lmi.cpp
>> Apparently the purpose of these changes is to avoid writing
>> empty xml child text nodes. Can you give me an example of the
>> problem that this change would solve? Is it simply a matter of
>> writing '<X/>' instead of '<X></X>' because the former seems
>> more tasteful?
> 
> As far as I can tell, yes.

Let me ask my US coworkers to apply those patch hunks and report
the results of their testing.

> (And shorter, too, and add_node() was 
> already used in other places, so it made sense to use it for 
> consistency alone when I saw this change.)

I wonder whether changing push_back() itself would be better
(we already modify the library via 'xmlwrapp-0.5.0.patch').
For instance, the patch would change 'ledger_xml_io.cpp' thus:

-            columns.push_back(xml::element("name", (*j).c_str()));
+            xml_lmi::add_node(columns, "name", *j);
-            columns.push_back(xml::element("title", title_map[*j].c_str()));
+            xml_lmi::add_node(columns, "title", title_map[*j]);
             supplementalreport.push_back(columns);

which makes the operation on the last (unchanged) line look
different from the preceding operations. Files 'xml_lmi*.?pp'
seem in retrospect to have been a serious design error on my
part, and it might be better to work toward getting rid of it.
But let me know if you feel strongly that this particular set
of changes should be applied now without considering whether
to refactor globally for simplicity.

BTW, this comment in 'inputillus_xml_io.cpp':
// XMLWRAPP !! The unit test demonstrates that the suppressed code is
// twenty-five percent slower. What would be really desirable is an
// (efficient) element-iterator class.
marks a block of code where I had to fall back on the xmlwrapp
api because the "virtualized" 'xml_lmi' api was too slow on a
crucial operation. So the virtualization layer is costly in
performance as well as in maintenance. And it's ugly, too.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]