Libo Sun
Libo Sun

Reputation: 11

Anything wrong with modify an object inside a loop?

public Collection<QueueMetrics> getAllGatewayQueueMetricsCurrent(String keywords)
{
    QueueMetrics qmInput = null;
    QueueMetrics qmImport = null ;
    QueueMetrics qmReport = null ;
    ArrayList<QueueMetrics> metrics = new ArrayList<QueueMetrics>();

    for (String keyword : keywords.split(",")){
        ConsoleServiceMBean console = getConsoleService(keyword);
        populateMessageCountStr(qmInput, keyword, console, "input");
        populateMessageCountStr(qmImport, keyword, console, "import");
        populateMessageCountStr(qmReport, keyword, console, "report");
    }
    metrics.add(qmInput);
    metrics.add(qmImport);
    metrics.add(qmReport);

    return metrics;
}

private void populateMessageCountStr(QueueMetrics qMetrics, String keyword, ConsoleServiceMBean console, String type)
{
    QueueMetrics tempMetric = console.getGatewayQueueMetricsCurrent(type, keyword);
            if( qMetrics == null ){
             qMetrics = tempMetric ;
    }else{
        qMetrics.setPendingMessageCountStr( qMetrics.getPendingMessageCountStr() + ", " + tempMetric.getPendingMessageCount());
         }

}

I tried to update qmInput, qmImport and qmReport inside the upper for loop, by calling the populateMessageCountStr and passing in the object itself to be updated. In next loop, the same object will be updated one more time with additional data. However, the end result of this program is that qmInput, qmImport and qmReport are all null. Anybody knows why? To provide more info, when I change the upper function a little bit:

        qmInput = populateMessageCountStr(qmInput, keyword, console, "input");
        qmImport = populateMessageCountStr(qmImport, keyword, console, "import");
        qmReport = populateMessageCountStr(qmReport, keyword, console, "report");

and the lower function returns the updated QueueMetrics instead of void. The program works as expcected. Anybody knows why the original way didn't work? I appreciate any useful comments.

Upvotes: 1

Views: 102

Answers (3)

Rohit Jain
Rohit Jain

Reputation: 213311

However, the end result of this program is that qmInput, qmImport and qmReport are all null.

Because, you are re-assigning the passed QueueMetrics reference in that method to a new object if it is equal to null, which it will be on the first invocation. In which case the original reference will still point to null. And hence it will remain null on all the further invocation of that method, as the condition qMetrics == null will always be true.

Since Java passes the reference by Value, re-assigning the qMetrics to a different instance, will change the value of the copy of the reference that is passed. But it will not modify the reference value of the original reference.

To solve the issue, simply return the modified reference from the method (change the return type of populateMessageCounterStr() to QueueMetrics, and assign it to the original reference on the calling side.

Upvotes: 2

ptyx
ptyx

Reputation: 4164

In java:

Object a = null;
Object b = a;
b = "foo";

leaves a null, which is your problem. What you want to do is:

Object a = <something>;
Object b = a;
b.mutate();

This is safe as b.mutate is mutating < something >, so effectively a.mutate() and b.mutate() are equivalent.

Upvotes: 0

Prashant Bhate
Prashant Bhate

Reputation: 11087

qMetrics is set locally and reference to it is lost. so caller of method populateMessageCountStr will not get to see the reference to it unless you return that reference.

if( qMetrics == null ){
             qMetrics = tempMetric ;

when changed it to

qmInput = populateMessageCountStr

and additionally change return type and return qMetrics in populateMessageCountStr

In first iteration when qMetrics is null new instance gets created console.getGatewayQueue... (and assuming it does not return null) qMetrics reference is returned and stored in qmInput and same is passed to populateMessageCountStr in subsequent iteration.

Upvotes: 0

Related Questions