HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Generating XML file from text input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filetextxmlinputgeneratingfrom

Problem

I am hoping the code is readable as is. I can probably just print the XML using streams but I have other reasons for using the library.

Please offer any inputs on how to improve program design/structure.

The header file

#ifndef GEN_XML
#define GEN_XML

#include 
#include 
#include 
#include 
#include 
#include 

typedef std::string stringType;
typedef std::vector stringVector;
typedef std::vector integerVector;

class gen_xml
{
private:
    int previousTime;
    int tempTime;
public:
    stringVector text_file;
    integerVector time_ms;
    stringVector text_info;
public:
    gen_xml():previousTime(1),tempTime(2) {};

    virtual int validate(int argc,char* argv[]);

    virtual void showInput(int argc,char* argv[]);

    virtual bool validateFileExtention(stringType fname,stringType ext);

    virtual int getAbsoluteTime(stringType value);

    virtual void getData(char* argv[]);

    virtual stringType toString(int num);

    virtual void generateXML(stringVector text_file,integerVector time_ms,char* FileName);

};

#endif  // GEN_XML


The CPP file

```
#include "tinyxml.h"
#include "tinystr.h"
#include "gen_xml.h"
using namespace std;

int main(int argc,char* argv[])
{
gen_xml req;
if(req.validate(argc,argv))
{
req.getData(argv);
req.generateXML(req.text_info,req.time_ms,argv[2]);
}
coutLinkEndChild(declaration);

TiXmlElement* msg;

TiXmlElement * root = new TiXmlElement( "tags" );
doc->LinkEndChild( root );
for(int i=0; iSetAttribute("event", "onCuePoint");
msgs->SetAttribute("overwrite", "true");
root->LinkEndChild( msgs );

msg = new TiXmlElement( "name" );

msg->LinkEndChild( new TiXmlText("CuePoint"+toString(i) ));
msgs->LinkEndChild( msg );

msg = new TiXmlElement( "timestamp" );
msg->LinkEndChild( new TiXmlText(toString((int)time_ms.at(i))));
msgs->LinkEndChild( msg );

msg= new TiXmlElement( "parameters" );
msgs

Solution

I would remove the validate method and do some modifications:

About validate:

  • Check argc and argv outside the class, just provide the filename to the getData method.



  • You also need to check when argc is less than 2, the user may forget to supply the filename.



  • Opening the file in the validate method and closing it again does not guarantee that it will be still available when you call getData, so it is better to open the file only there and handle any errors there.



About getData:

  • I would rename it to loadData.



  • For each line loaded you insert a value in time_ms and text_info vectors, call reserve before the loop to pre-allocate the needed memory.



  • No need to call close on the stream, it will close when its scopes end.



About validateFileExtention:

  • Use rfind instead of find and just grab the last "." from the string.



  • I would instead create a function to copy the fileExtension and check both extensions at once, instead of finding the extension each time.



  • I would also not even bother about file extension at all. I would just let the parser validate it.



About generateXML:

  • Memory leak: you never delete doc. Also, if you need it just inside the method, why you use dynamic memory? Just create it on stack.



  • Also, check if tinyxml deletes everything that you insert on the document. If not, you will have to handle this too.

Context

StackExchange Code Review Q#1608, answer score: 5

Revisions (0)

No revisions yet.