I am having a stylistic discussion with a non-ABL person. His advice is to do as little work in the constructor as possible because constructors are inherently fragile so one doesn't want any possibility of an error arising for any other reason than that the class failed to instantiate.
The context of the question is a class something like this:
class DoWork:
constructor public DoWork():
DoStepOne().
DoStepTwo().
DoStepThree().
end constructor.
method private void DoStepOne():
....
end method.
method private void DoStepTwo():
....
end method.
method private void DoStepThree():
....
end method.
end class.
I.e., it is a class that once instanciated (with or without a passed in parameter) and it does some work in a fixed order and then it is done and can be deleted. His inclination is to either add a public method DoSteps() which calls the three private methods or to make the private methods public and call them in the program using the class. The latter doesn't appeal to me because it introduces the possibility of error in executing the steps in the wrong order or incompletely. The former doesn't appeal to me because it requires an extra call that adds nothing useful to the functionality. But, using a DoSteps() method could be justified if the nature of the constructor was such that one could not handle errors that might arise in the steps because a complete object did not yet exist.
Thoughts? Actual experience?
BTW, the same issues potentially exist for a class which does stick around. E.g., the fixed steps build some kind of list and then there are additional public methods for navigating the list, but building the list is entirely deterministic.
Referring to your own preference for Events for logging and process display purposes:
You cannot have event subscribers in the Constructor, because you need to come back from the Constructor to a Caller to be able to Subscribe() to an event. Unless you pass in an object reference as an input parameter to the constructor and basically revert who is Subscribing whom to what...
So I'd go for a single worker method, but give it a pretty much standarized name, like Run() . When it's more a kind of initialization (your last sentence), if it's substantial amount of work, I'd put it into a Public Init() or Reset() method when it's redoable, and not in the Constructor.
I don't like DoSteps() as a name because the caller shouldn't know if the work he wants to have done requires one or more than one step or steps (singular or plural) - you said it's a discussion about style.
Multiple public methods should only be exposed when it's expected that the Caller needs to do something between the steps. Or the Caller should be in control of the order of processing.
When there is a chance that the class can do the same work more than once, a Reset() method would be my preference.
Consider this discussion independent of the one on heartbeat or logging .... I can't say that I have a preference there yet anyway ... still exploring pros and cons.
If the work was something that could be redone, I would have no question that should be a public method because one has to have something to call again and again. The context is more one-time uses like an initialization or something where one would throw away the object and start with a new one rather than reuse the same object.
DoStepOne() was not intended as an actual method name!
The question is, is there some solid practical reason to do:
MyObj = new Whatever().
MyObj:Run().
or whatever instead of simply doing the work in the constructor?
I think there's a problem with the following statements:
MyObj = new Whatever().
MyObj:Run().
If I were the class containing the statements above, I could also choose to NOT call the Run method. Then you would have to check within the Whatever class whether the Run method was actually called. I don't think that the implementation of the initializationbehavior of a class should be the responsibility of the class that does the instantiation.
All methods and properties that are exposed by class Whatever should function independently from each other. And if not, you obviously need to check for that. This also applies to the above statements, but is unnessecary.
This kind of initialization behavior, if recurring, is often implemented using the Template method:
constructor public doWorkParent ( ):
DoStepOne().
DoStepTwo().
DoStepThree().
end constructor.
method protected abstract void DoStepOne().
method protected abstract void DoStepTwo().
method protected abstract void DoStepThree().
end class.
class doWork inherits doWorkParent:
constructor public doWork():
super().
end constructor.
method protected override void DoStepOne():
/* step 1 */
end method.
method protected override void DoStepTwo():
/* step 2 */
end method.
method protected override void DoStepThree():
/* step 3 */
end method.
end class.
I don't think that the implementation of the initializationbehavior of a class should be the responsibility of the class that does the instantiation.
Right for the initialization of a class. But the question was where to stick work that a class does. So I think we just need more information about the actual use case:
Is the work done to be considered as initialization of the called class or redoable work on behalf of consumers.
There are many other things to consider:
Event subscriptions as I have mentioned before are typically only possible once returned from the constructor.
Error handling: An error thrown from the constructor voids the instance. So what behavior is desired when the work fails and needs to signal that though an error. I'd rather receive an error object thrown from a worker method then having to query a Failed method from an instance (which would also require to remember doing an additional step in the consumer, making the protocol more complex)
Output: How is one supposed to get a result of that work. OUTPUT Parameters from the constructor? I'm not 100% sure if that's even possible, but at least it would be damn ugly. A public worker method might return any kind of parameters.
I probably missed a bunch of other obvious things to consider... But it's certainly not a yes/no or black/white kind of question. There are many aspects that may result to different results in different constellations.
In the cases I am looking at, I see no reason for introducing inheritance. There is only one concrete class.
OK, I have the message that I can't use event subscriptions if I put the work in the constructor. I'm not sure this is an issue, however, for two reasons:
1. I am more likely, if anything, to want to publish an event from a class of this type than to subscribe to one.
2. The recent discussion of events was for keeping track of the progress of a long running iterative process, which does not apply here.
Error handling: I understand that an error will result in no object instance. I'm not sure that I see that as a real problem since a failure in the DoIT() would leave me with an object that had not done its work. Can I throw an error from a failed constructor?
As far as output, not every class needs output per se. In the CreateContext example the effect of running the class is to initialize three .NET singletons and provide them with some appropriate values for use by Proparse. So, there, the output is indirect. As I mentioned, the question also applies to classes where there is initialization work done which results in some internal information but there are then methods to use that information. E.g., I have a little routing for dividing a full path name into a propath part and a reference part. This includes an initialization to create a temp-table with the propath components that is referenced by the calling methods. There is no output per se from the initialization, but there is a useful byproduct of the initialization. I agree that output parameters on constructors is ugly, even if legal.
I'm not sure that I see that as a real problem since a failure in the DoIT() would leave me with an object that had not done its work.
That's right - but depending on the cause of the error, the caller may fix the issue (with different parameters to the worker method of changes to the wider environment) and try again on the same instance.
You are hiding all the details from us to make this a worthwhile discussion
> Can I throw an error from a failed constructor?
Any error you like.
OK, let's look at a specific example. To do Proparse scripting, one has to establish some context. So, I created a CreateContext class that has two methods. One method initializes three .NET singletons which will be used by Proparse and then assigns some values to one of those singletons. The other method is responsible for dumping the schema from any attached databases into a text file and then calling a method on one of the singletons to load that schema for access by Proparse. The dump is done in a loop setting the DICTDB alias for each database and then calling another object which dumps the schema of that database to append to the text file. So, in the CreateContext case we have two methods which are executed in a specified order and done once per session. In the DumpOneSchema case one opens a stream and calls one method and then closes the stream. In both cases, if something fails one is into debugging ... nothing to try to fix or retry or try different parameters ... just failed and can't go forward.
When there is a chance that the class can do the same work more than once, a Reset() method would be my preference.
I really like the Initialize() approach - and I've been long looking for a name for the companion method: thanks for Reset(), Mike.
AutoEdge|TheFactory uses this approach, and even goes one step further to have CreateComponent/DestroyComponent in addition to the constructor/destructor and Initialize/ method pairs. Some info at http://communities.progress.com/pcom/docs/DOC-105067#Object_initialization .
-- peter
tamhas wrote:
Error handling: I understand that an error will result in no object instance. I'm not sure that I see that as a real problem since a failure in the DoIT() would leave me with an object that had not done its work. Can I throw an error from a failed constructor?
There is no output per se from the initialization, but there is a useful byproduct of the initialization. I agree that output parameters on constructors is ugly, even if legal.
Useful byproduct of the initialization? That's not very OO, now is it - what happened to the single responsibility priniciple?
Broadly, object lifecycle and behaviour are difference concerns as far as I'm concerned. when you NEW() or Factory:Get() or something similar an object, you should have a clean, empty (basically) object that you can now work on. Making the constructor do work is not something I like to do (and yes, I know the GUI for .NET objects do exactly that, but that's done now).
-- peter
Right, but understand that this is not your usual object ... The specific one under consideration exists only to establish a context. The context which it is creating is in the form of .NET singletons which will be used by Proparse. It itself has no information.
pjudge wrote:
Broadly, object lifecycle and behaviour are difference concerns as far as I'm concerned. when you NEW() or Factory:Get() or something similar an object, you should have a clean, empty (basically) object that you can now work on. Making the constructor do work is not something I like to do (and yes, I know the GUI for .NET objects do exactly that, but that's done now).
-- peter
guess most of us don't expect an object to actually do anything at initialization time... if the 3 methods need to be called in order then expose just one public method that does just that, still able to call that on a single `new DoWork():DoWork().` or make the DoWork method return this-object and use fluid syntax.
beside the reasons already mentioned against such an approach I wonder how do you think writing test-cases for that will look like?
In this case, any test case would have to be written in .NET, I think. So, I just plugged it in to replace the procedural code it was taken from in a working example.
I think the most important consideration is what the user of the object has to know. IOW, above all you must do good API design.
Still, there are always tradeoffs to be made. Your non 4GL person does have a point and is not always wrong. If the constructor ends up being so complicated that it is likely to fail, that argues for moving stuff out of the constructor into some other method. But if you have to do that, then maybe the design is all wrong.
I suppose that simplicity of use is one of the things that got me going in this direction. E.g., the launch routine I am using at the moment (fancier API yet to come) needs to set up the Proparse context .NET singletons, built a list of compile units below some specified directory, and then run the scaner on that file list. This comes down to:
/* Set up Proparse Context and Schema */
MyContext = new CreateContext().
/* Get files to process */
FilesToScan = new CUFilesInDirectory( chDirectory ).
obMyScan = new ScanFileList( FilesToScan ).
obMyScan:ScanAllFiles().
Both CreateContext and CUFilesInDirectory are highly unlikely to fail and are simple, deterministic processes. So, moving the things around to separate the new from the work seems to be making them harder to use. And, if they do fail, it it debugger time, not something one can fix and try again.
For the record, I have come around to deciding to use Initialize() and, where relevant, Reset() methods and do minimum work in the constructor.