0

Today at work I had a discussion with 2 developers regarding the way to add an element into a list. They told me that this is the more efficient way:

final FileInfo fileInfo = new FileInfo();
for (GroupInfo groupInfo : cft.getGroupFiles()) {
    fileInfo.setClassName(groupInfo.getClassGeneration());
    fileInfo.setFileId(groupInfo.getId().getCfgFileTypeId());
    fileInfoList.add(fileInfo);
}

But I'd always done it in a slightly different way:

for (GroupInfo groupInfo : cft.getGroupFiles()) {
    FileInfo fileInfo = new FileInfo();
    fileInfo.setClassName(groupInfo.getClassGeneration());
    fileInfo.setFileId(groupInfo.getId().getCfgFileTypeId());
    fileInfoList.add(fileInfo);
}

So, what is the most efficient way to do this? Why? I think that the first way uses less memory than the second way, but the second way generates a new instance and for an instance, a new memory-reference and then adds it to the list, but I had bad experiences using the first way when I had to convert or use some elements from the list using the first way...So, can you help me with this? Thanks!

Thomas Shields
  • 8,874
  • 5
  • 42
  • 77
  • 8
    the first 'method' will end up with a list filled with the same `FileInfo` object with the properties of the last `GroupInfo` -- I doubt that is intended – obataku Jul 06 '15 at 19:44
  • 3
    first way, is not actually at all a way :) , At the end of loop you will end up having same object throughout the list with the last values of loop. – Amit.rk3 Jul 06 '15 at 19:45
  • 3
    You're coworkers prefer broken code just for the imagine, that this might be faster? This isn't even important, since JIT would take care about optimization here. – Tom Jul 06 '15 at 19:53
  • 2
    I hope that isn't what they really told you, otherwise you need two new co-workers. More likely they told you to declare the variable outside the loop and assign it inside. But if they think that's faster, they're dreaming. – user207421 Jul 06 '15 at 20:01

2 Answers2

1

In the first example you're mutating then adding the same instance of FileInfo. Thus you'll end up with fileInfoList referencing the same instance of FileInfo, which will have the state of the last loop iteration. I'm pretty sure this is not the desired behavior. As usual, correctness supersedes efficiency.

Steve Kuo
  • 61,876
  • 75
  • 195
  • 257
0

Below list will only have one element:

final FileInfo fileInfo = new FileInfo();
for (GroupInfo groupInfo : cft.getGroupFiles()){
    fileInfo.setClassName(groupInfo.getClassGeneration());
    fileInfo.setFileId(groupInfo.getId().getCfgFileTypeId());
    fileInfoList.add(fileInfo);
}

And below will have as many are in cft.getGroupFiles() list.

final FileInfo fileInfo = new FileInfo();
for (GroupInfo groupInfo : cft.getGroupFiles()){
    FileInfo fileInfo = new FileInfo();
    fileInfo.setClassName(groupInfo.getClassGeneration());
    fileInfo.setFileId(groupInfo.getId().getCfgFileTypeId());
    fileInfoList.add(fileInfo);
}

So we can't compare both methods on the basis of memory efficiency as each one is a different implementation.

Danielson
  • 2,605
  • 2
  • 28
  • 51
Arpit Aggarwal
  • 27,626
  • 16
  • 90
  • 108
  • This answer is redundant after reading the comments. Perhaps we should wait for clarification from OP. Maybe they missed some code. – CubeJockey Jul 06 '15 at 20:03