Search code examples
oopvga

Is it OK to create an object inside a function


I work on a class in VBA, that encapsulates downloading stuff with MSXML2.XmlHttp.

There are three possibilities for the return value: Text, XML and Stream.

Should I create a function for each:

 aText=myDownloader.TextSynchronous(URL,formData,dlPost,....)
 aXml.load myDownloader.XmlSynchronous(URL,formData,dlPost,....)

Or can I just return the XmlHttpObject I created inside the class and then have

 aText=myDownloader.Synchronous(URL,formData,dlPost,.....).ResponseText
 aXML=myDownloader.Synchronous(URL,formData,dlPost,.....).ResponseXML

In the former case I can set the obj to nothing in the class but have to write several functions that are more or less the same.

In the latter case, I relay on the "garbage collector" but have a leaner class.

Both should work, but which one is better coding style?


Solution

  • After much seeking around in the web (triggered by the comment by EmmadKareem) I found this:

    First of all, Dont do localObject=Nothing at the end of a method - the variable goes out of scope anyway and is discarded. see this older but enlightening post on msdn

    VBA uses reference counting and apart from some older bugs on ADO this seems to work woute well and (as I understand) immediately discards ressources that are not used anymore. So from a performance/memory usage point of view this seems not to be a problem.

    As to the coding style: I think the uncomfortable fdeeling I had when I designed this could go away by simply renaming the function to myDownloader.getSyncDLObj(...) or some such.

    There seem to be two camps on codestyle. One promotes clean code, which is easy to read, but uses five lines everytime you use it. Its most important prerogative is "every function should do one thing and one thing only. Their approach would probably look something like

    myDownloader.URL="..."
    myDownloader.method=dlSync
    myDownloader.download
    aText=myDownloader.getXmlHttpObj.ResponseText
    myDownloader.freeResources
    

    and one is OK with the more cluttered, but less lineconsuming

    aText=myDownloader.getSyncObj(...).ResponseText
    

    both have their merits both none is wrong, dangerous or frowned upon. As this is a helper class and I use it to remove the inner workings of the xmlhttp from the main code I am more comfortable with the second approach here. (One line for one goal ;)

    I would be very interested on anyones take on that matter