Calling member functions inside constructor

[ktsangop]

Hi everyone!

I am trying for the first time to write "proper" object oriented c++ (ANSI) code.
Up until now everything was made easy to me using visual studio and working with mfc or .net which both handle a lot of the "dirty" stuff...

To the point : I want to build a class that all it's member variables are initialized by parsing an xml file (seperate file for each object).
To parse xml files i use libxml2 library http://xmlsoft.org/.
I have already made a function to parse the xml files and store the data to my variables and this is going to be a member function of my class.

Now how should i design my class for better functionality - portabiliy?
Should i call the xml parse function inside the constructor of my class or leave the constructor "clean" and call the function outside class declaration?

Is there anything i should consider before using the one way or the other?

Thanks in advance.

[L7Sqr]

Consider how you want to use the class. Do you want to construct the object with a specific XML file similar to Object("some_file.xml") ? Or do you want to map a file to the object through a public member like obj->load_config ("some_file.xml"); Nothing is stopping you from from providing the public function (or private, if you prefer) and calling that from within the constructor. One thing to consider, however, is how you will deal with errors associated with loading the file in your constructor. There is no return value to signal success/fail. Will you throw an exception, or set a flag in the class?

[L7Sqr]

Consider how you want to use the class. Do you want to construct the object with a specific XML file similar to Object("some_file.xml") ? Or do you want to map a file to the object through a public member like obj->load_config ("some_file.xml"); Nothing is stopping you from from providing the public function (or private, if you prefer) and calling that from within the constructor. One thing to consider, however, is how you will deal with errors associated with loading the file in your constructor. There is no return value to signal success/fail. Will you throw an exception, or set a flag in the class?

[ktsangop]

What i had in mind is the Object("some_file.xml").
Since there is nothing to consider, i will probably use it, and call load_config in the constructor.
As for error handling load_config should return an error value (to a public variable) if it cannot parse the file and i will check the value like this:

myclass Object("some_file.xml");
if (Object.error_val == -1)
     delete Object;
     //or raise exception or terminate program...don't know yet!

[mike_2000_17]

This code:

myclass Object("some_file.xml");
if (Object.error_val == -1)
     delete Object;

Is invalid. You cannot "delete" things that are allocated on the stack. "delete" can only be used to delete things that were allocated with "new", as so:

myclass* Object = new myclass("some_file.xml");
if (Object->error_val == -1)
     delete Object;

Second thing is, you should throw an exception in the constructor if it was not successful. That's the best way to go. I would recommend that your function load_config throws an exception if the xml file cannot be loaded or is corrupt (not the correct format or the right data in it). In this case, you can do code like this:

try {
  myclass Object("some_file.xml");

  //use the Object, assuming it was correctly loaded (otherwise it would have thrown). 
  //...

} catch(cannot_load_xml_file& e) {
  // report error or do something alternative (backup solution).
};

Or, for dynamically allocated object:

try {
  std::auto_ptr<myclass> Object = std::auto_ptr<myclass>(new myclass()); //default constructor and automatic pointer (in C++0x, use std::unique_ptr, or std::shared_ptr).
  Object->load_config("some_file.xml");

  //use the object, assuming it was created and loaded successfully.
  //..

} catch(std::bad_alloc& e) {
  // report error about dynamic allocation failing to work.
} catch(cannot_load_xml_file& e) {
  // report error or do something alternative (backup solution).
};

Returning error codes or flags is an outdated method that is not recommended unless you are writing C code or writing functions that are exported from a DLL or .so file.

If you are serious about loading/saving object data to a file, you should look into the topic of "serialization", that's the formal name for this. Look, for a good example, at the Boost.Serialization library.

[ktsangop]

Returning error codes or flags is an outdated method that is not recommended unless you are writing C code or writing functions that are exported from a DLL or .so file.

I might have to implement it as a DLL so not sure right now about what kind of error detection i am going to use.

If you are serious about loading/saving object data to a file, you should look into the topic of "serialization", that's the formal name for this. Look, for a good example, at the Boost.Serialization library.

Thanks for the suggestion. I haven't thought of serialization so i should read libxml's documentation thoroughly to see how to address that issue.