Opening and validating an XML file

Question

I have a section of code that repeats many lines and I am looking to refactor this code possibly so that a function can be used instead of the same lines of code each time. Below is the section of code and the repeating code occurs in each if statement

Traversal::Traversal(string source, bool isFileName)
 {
 using namespace XML_Parser;
 using std::endl;
 std::ostringstream ppd;
 if (isFileName)
 {
 string content, line;
 std::ifstream file(source);
 if (! file)
 {
 ppd << endl << "error opening file " << source;
 cRep = ppd.str();
 cSuc = false;
 return;
 }
 ppd << endl << " file " << source << " opened ";
 while (file.good())
 {
 getline(file, line);
 content += '\n';
 content += line;
 }
 ppd << endl << "file " << source << " read ";
 source = content;
 }
 if (! elementExists(source,"afcd"))
 {
 ppd << endl << "no afcd tag";
 cRep = ppd.str();
 cSuc = false;
 return;
 }
 if (! elementExists(source,"rteirl"))
 {
 ppd << endl << "no rteirl tag";
 cRep = ppd.str();
 cSuc = false;
 return;
 }

How would I go about making this cleaner?

• molbdnilo: while (file.good()) is wrong for the same reasons that while (!file.eof()) is.

Answer 1

The basic aim is to put repeating parts in the function, while passing as a parameter anything that's different:

if (elementMissing(source, "afcd")) {
 return;
}
if (elementMissing(source, "rteirl")) {
 return;
}
bool elementMissing(source, element) {
 if (! elementExists(source, element)) {
 ppd << endl << "no " << element << " tag";
 cRep = ppd.str();
 cSuc = false;
 return true;
 } else {
 return false;
 }
}

I'm not sure what cRep and cSuc are supposed to be doing, so I can't help you with that part.

Answer 2

Use a helper function, something along the line of:

bool Helper(std::string tagtype)
{
 if (! elementExists(source, tagtype))
 {
 ppd << endl << "no " << tagtype << " tag";
 cRep = ppd.str();
 cSuc = false;
 return true;
 }
 return false;
}

obviously you need to figure out how to pass in (by reference) the values of ppd, cRep, etc.

Answer 3

How about:

void
Traversal::BadStatus( std::ostringstream& ppd, const string& msg )
{
 ppd << endl << msg;
 cRep = ppd.str();
 cSuc = false;
}

Then:

if( ! elementExists(source, "asdf") ) {
 BadStatus("no asdf tag");
 return;
}

Usually, though, the best solution to an error at construction time is to throw an exception.