patterncppMinor
Generating XML file from text input
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
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
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_XMLThe 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
About
About
About
About
validate:- Check
argcandargvoutside the class, just provide thefilenameto thegetDatamethod.
- You also need to check when
argcis less than 2, the user may forget to supply thefilename.
- 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
reservebefore 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
rfindinstead offindand just grab the last "." from the string.
- I would instead create a function to copy the
fileExtensionand 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.