Where to do work (Constructor, Init, etc.)

While the answer depends on the individual circumstance, I think the principles are consistent.

Only do as much work in the constructor as is necessary to set up the object for use, including no actions which have the potential to fail.

If there is additional work needed to get the object ready, do that in the Init method where it is possible to throw an error and still have a valid object.  Among other things, this allows you to correct the error and properly init the object when that makes sense.

Other work goes in other methods ... which vary from DoItAll to DoOneThing according to the purpose of the object.

afutrel wrote:
Moving this discussion from PEG to here so that I can get more eyes and thoughts.
My initial question was about having the constructor fail and thus the consumer would have an invalid object. Then there were discussions saying to move most work to init. And then even further discussion about possibly not even needing init, but individually do things.
My overall question is, if I want to simplify the design and keep the consumer of the class from hurting themselves, should things be done in the constructor/init, or should one just rely on the user of the class to use it properly?
This all started from a decision to move a small portion of an app to objects. One object that I will write will allow the user to write to a "log". The log itself will be database rows, but to set up the log, I want to make sure that other things in the db are already set up.
It's basically the log for jobs that will import/export files. The simple overview is something like:
DBTable "Interface"
  Field - InterfaceName
  Field - ExternalFile
DBTable "Log"
  Field InterfaceName
  Field LogDateTime
  Field Message
So my class is something like:
Class myClass:
  def public property InterfaceName as char
    get .
    private set .

Constructor Public myClass(pInterface as char):
  Check here if pInterface exists in DBTableInterface.
  If not, fail to instantiate object
End Constructor.

Method Public void writeLog(pMessage as character) 
     Write DBTableLog row using property InterfaceName, pMessage, Now.
end method.

end class.
So the question came up, should I do the above work/checks in the Constructor. Or should it be in an init. Or, should I just have a public "setter" for the "pInterface" property and have that setter do the work. Then, if I use the setter to do the work, how much additional work do I need to put into method "writeLog" to make sure the class is correct before it can do it's work.
That's a very oversimplified view of the thing, but it's the overall idea of the class. In reality, there are other db table-rows that I need to verify exist (one that defines directories, one that defines external consumers, etc.) And then there will be a longer key to the "Log" table (more components that is) and several different methods to write log messages, and a "Success" method and a "Fail" method that will also do some additional work.
My underlying question though is validating things in the class before the object starts being used.  I could assume that the consumer of the class would use it properly and only instantiate it with good data and just not worry about cross table dependencies. Or should my class only let you do "right" things. That's kind of my question. And assuming that the class should enforce "right" things, where should the work of that enforcement take place?
Wide open to thoughts and ideas. So far just discussing it has given me a lot of thoughts about things and actually I may end up with more than one class to handle what I want so that each is very specific to the work it does.
Thanks in advance (and thanks thus far for opions and thoughts already offered.)
Andy

If pInterface is a real interface (ie a type), then I'd pass it as Progress.Lang.Class. You can extract the name of the type easily enough from that (via the TypeName property).

We have some of the coding conventions we used for our sample app, including what to put in constructors at  http://communities.progress.com/pcom/docs/DOC-105067. There's a link in there to the MSDN Constructor design page at http://msdn.microsoft.com/en-us/library/ms229060.aspx. We didn't follow all of those guidelines, and it's another perspective on the matter.

My underlying question though is validating things in the class before the object starts being used.  I could assume that the consumer of the class would use it properly and only instantiate it with good data and just not worry about cross table dependencies. Or should my class only let you do "right" things. That's kind of my question. And assuming that the class should enforce "right" things, where should the work of that enforcement take place?

Our approach is to set mandatory properties via constructor. Make sure they've got valid values if necessary, but don't do any 'real' work. Then we have an Initialize() method which does any work. The important thing to me is to make sure that once you get an object back from the factory (whether that's a simple static method or something more complex) that it is in a usable state. For me, this means that mandatory properties are public|protected get and PRIVATE set, with the value being set in the constructor.

It also depends somewhat on what you mean by 'right'. Is it just a case of having valid values set? Or is there more complex logic involved?

Something as simple as the below can help with that. It should also be reasonably easy to find cases where NEW MyClass is called in the rest of the codebase, and replace it with the factory method.

Class myClass:
    /* factory method should be only way to instantiate MyClass ever, anywhere in code */
    method static public MyClass ClassFactory (pInterface as char):
       def var oInstance as MyClass.
       
       oInstance = new MyClass(pInterface).
       oInstance:Init().
       
       return oInstance.
    end method.

-- peter

pjudge wrote:
 
Our approach is to set mandatory properties via constructor. Make sure they've got valid values if necessary, but don't do any 'real' work. Then we have an Initialize() method which does any work. The important thing to me is to make sure that once you get an object back from the factory (whether that's a simple static method or something more complex) that it is in a usable state. For me, this means that mandatory properties are public|protected get and PRIVATE set, with the value being set in the constructor.
It also depends somewhat on what you mean by 'right'. Is it just a case of having valid values set? Or is there more complex logic involved?
Something as simple as the below can help with that. It should also be reasonably easy to find cases where NEW MyClass is called in the rest of the codebase, and replace it with the factory method.
Class myClass:

 

    /* factory method should be only way to instantiate MyClass ever, anywhere in code */

    method static public MyClass ClassFactory (pInterface as char):

       def var oInstance as MyClass.

       

       oInstance = new MyClass(pInterface).

       oInstance:Init().

       

       return oInstance.

    end method.

 







-- peter

First "Interface" here in my example is actually just an interface to an external system. I'm wrapping up import/export routines to share data with other internal systems.

So as far as real work. Basically, my object will know what directory to put things in. But only if you pass in a valid "Interface" to the object. It uses that to look in the db to determine where that "Interfaces" files should go.  Mostly this should be just a discussion because it's developers setting up the db records and those same developers using the object, but... just in case you were to have a missing directory structure, I want the object to behave rationally.

I'm wondering if you should get a valid object back or not, if things aren't set up correctly.

So taking your factory example, I can certainly move most of my "work" to the init and have the constructor really just set up properties, etc. so my "fail" code is in my init. But I guess my question would be, assuming the init found something wrong or whatever, should you return that instance? Or should you throw an error?

So would it be more like:

  oInstance = new MyClass(pInterface).

  if not oInstance:Init() then undo, throw someerror.

  else return oInstance.

Many thanks for all of the input. It's certainly a different way of thinking and I don't know that there's really a wrong way to do it with the current class I'm working on. But I'm really just trying to get a good feel for how others do things, best practices, etc.

I could assume that the consumer of the class would use it properly and only instantiate it with good data

You could. Every once in a while this assumption would turn out to be correct.

gus wrote:
I could assume that the consumer of the class would use it properly and only instantiate it with good data
You could. Every once in a while this assumption would turn out to be correct.

  Still laughing...

If it's impossible to do anything else if input is wrong: CONSTRUCTOR, and THROW an exception if the input is not valid.

If data will be provided later: Code the PROPERTY definition to put the class in the proper context when the property is changed, and THROW an exception if the value in not usable.

If you are initializing a UI (ex. Form, UserControl): LOAD event is the best place

jquerijero wrote:
If it's impossible to do anything else if the input is wrong: CONSTRUCTOR
If data will be provided later: Code the PROPERTY definition to put the class in the proper context when the property is changed, and THROW an exception if the value in not usable.
If you are initializing a UI (ex. Form, UserControl): LOAD event is the best place

That seems to be a good rule of thumb and basically what I came to.  Thanks to everyone for input. I guess it's like everything else, it comes down to design decisions. I was trying to wrap a "log writer" for external system interfaces and we have tables that define those interfaces, I was torn about not letting you create a "log" for an interface that wasn't set up properly. In the end, I decided, sure, I'll let you make the log, but then log to that log that your interface isn't defined properly. So once I got my head around that decision, it was easier to move things out of the constructor. So the constructor now will let you create a "not-quite-right" object and that portion will work even though it's not technically "right". Then we can report on the "not-right" log files since they will have errors.

Overall, the discussion helped me tremendously, especially thinking about the problem space.

I'm still open to thoughts and opinions on things to read, best practices (especially OOABL related), etc.

Thanks again to all.

andy