Reputation: 9194
I am connecting to an IBM Websphere MQ server in my .Net code and I wanted to make sure that I am following best practice when using "finally".
I currently have the below code block which I believe can be modified to just have the close portion in the finally clause. Is that correct? (I am catching errors in the calling portion of the application).
Hashtable properties = new Hashtable();
properties.Add(MQC.TRANSPORT_PROPERTY, MQC.TRANSPORT_MQSERIES_CLIENT);
properties.Add(MQC.CHANNEL_PROPERTY, channel);
properties.Add(MQC.HOST_NAME_PROPERTY, host);
properties.Add(MQC.PORT_PROPERTY, port);
MQQueueManager qmgr = new MQQueueManager(queueManager, properties);
try
{
var queueDepth = qmgr.AccessQueue(userQueue,
MQC.MQOO_INPUT_AS_Q_DEF +
MQC.MQOO_FAIL_IF_QUIESCING +
MQC.MQOO_INQUIRE).CurrentDepth;
if (qmgr.IsOpen)
qmgr.Close();
return queueDepth;
}
finally
{
if (qmgr.IsOpen)
qmgr.Close();
}
Is now this
Hashtable properties = new Hashtable();
properties.Add(MQC.TRANSPORT_PROPERTY, MQC.TRANSPORT_MQSERIES_CLIENT);
properties.Add(MQC.CHANNEL_PROPERTY, channel);
properties.Add(MQC.HOST_NAME_PROPERTY, host);
properties.Add(MQC.PORT_PROPERTY, port);
MQQueueManager qmgr = new MQQueueManager(queueManager, properties);
try
{
var queueDepth = qmgr.AccessQueue(userQueue,
MQC.MQOO_INPUT_AS_Q_DEF +
MQC.MQOO_FAIL_IF_QUIESCING +
MQC.MQOO_INQUIRE).CurrentDepth;
return queueDepth;
}
finally
{
if (qmgr.IsOpen)
qmgr.Close();
}
EDIT: Renan made a good suggestion. I didn't think the MQQueueManger was disposable. Sounds like I could potentially do this:
using(MQQueueManager qmgr = new MQQueueManager(queueManager, properties))
{
var queueDepth = qmgr.AccessQueue(userQueue,
MQC.MQOO_INPUT_AS_Q_DEF +
MQC.MQOO_FAIL_IF_QUIESCING +
MQC.MQOO_INQUIRE).CurrentDepth;
return queueDepth;
}
Edit: I did some research after reading Renan's suggestion and found the below. Sounds like they did in fact make it disposable.
Upvotes: 0
Views: 2290
Reputation: 7456
First of all, there is no valid reason that an application needs to know the depth of a queue. Applications should process ALL messages in the queue until it is empty.
Secondly, do NOT use IsOpen methods as they do not work as you might expect. The IsOpen method does not actually check if the queue handle is open - it only checks an internal flag. Hence, do not use it.
Third, you do NOT close a queue manager object but rather you disconnect from a queue manager.
Fourth, when connecting to a queue manager, that statement needs to be in a try/catch because it will throw an MQException if the connection fails.
Here is a better layout of the code which will catch and handle errors:
MQQueueManager qMgr = null;
MQQueue queue = null;
int openOptions = MQC.MQOO_INPUT_AS_Q_DEF + MQC.MQOO_FAIL_IF_QUIESCING + MQC.MQOO_INQUIRE;
try
{
qMgr = new MQQueueManager(qMgrName);
System.Console.Out.WriteLine("Successfully connected to " + qMgrName);
queue = qMgr.AccessQueue(qName, openOptions, null, null, null);
System.Console.Out.WriteLine("Successfully opened " + qName);
System.Console.Out.WriteLine("Current queue depth is " + queue.CurrentDepth);
}
catch (MQException mqex)
{
System.Console.Out.WriteLine("Exception CC=" + mqex.CompletionCode + " : RC=" + mqex.ReasonCode);
}
catch (System.IO.IOException ioex)
{
System.Console.Out.WriteLine("Exception ioex=" + ioex);
}
finally
{
try
{
if (queue !=null)
{
queue.Close();
System.Console.Out.WriteLine("Successfully closed " + qName);
}
}
catch (MQException mqex)
{
System.Console.Out.WriteLine("Exception on close CC=" + mqex.CompletionCode + " : RC=" + mqex.ReasonCode);
}
try
{
if (qMgr !=null)
{
qMgr.Disconnect();
System.Console.Out.WriteLine("Disconnected from " + qMgrName);
}
}
catch (MQException mqex)
{
System.Console.Out.WriteLine("Exception on disconnect CC=" + mqex.CompletionCode + " : RC=" + mqex.ReasonCode);
}
}
Upvotes: 2
Reputation: 414
You are right. The finally clause will execute even if the code in the try block returns an exception.
You could also use the "using" construct for the connection (if it implements IDisposable, which it should).
using(qmgr){
//do stuff
}
Upvotes: 4
Reputation: 65049
That is fine.
A finally
block is guaranteed by the CLR to be called (except in some very very rare edge cases, which IIRC are internal CLR errors, such as a call to FailFast
or an ExecutingEngineException
). By removing it from the try
, you are removing redundant code.
Upvotes: 2