3

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?

  • Can you really rely on garbage collector in VBA? – NoChance Apr 19 '12 at 12:06
  • @EmmadKareem: that pointed me in the right direction. VBA uses reference counting and (apart from some old bugs in ADO there seem to be no problems as long as you dont do circular referencing. –  Apr 19 '12 at 14:45

2 Answers2

0

In my opinion, the first way is better because you don't expose low level details to a high level of the abstraction.

I did something similar with a web crawler in Java, so I have a class only to manipulate the URL connection getting all the needed data (low level) and a high level class using the low level class that return an object called Page.

You can have a third method that only execute myDownloader.Synchronous(URL,formData,dlPost,.....) and stores the returned object in a private variable and the others method only manipulate this object. This form, you will only open the connection one time.

Renato Dinhani
  • 35,057
  • 55
  • 139
  • 199
  • Which would lead to: myDownloader.DoDownloadSynchronous(...) / aText=myDownloader.getText and the internal XmlHttp is kept alive as long as the Downloader lives... –  Apr 19 '12 at 14:39
  • @Johanness After doing all the processing needed, remove the reference. The private var is used to avoid reopening the connection, but is needed only if you get more than one type of return. If not, it is not needed. – Renato Dinhani Apr 19 '12 at 14:50
0

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

Community
  • 1
  • 1