• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 239
  • Last Modified:

Is this good practice? - Create an object instance in a loop

Hello

I am relatively new to C#, but have done a bit of Java in the past. I can not remember however if this is good practice?

I have this code

foreach(string fileName in fileEntries)
{
ArrayList fileDetailsArrayList = new ArrayList(); ***************
                  
FileInfo fi = new FileInfo(fileName);
long fileSize = fi.Length;
                  
//put the name and size into an ArrayList
fileDetailsArrayList.Add(fileName);
fileDetailsArrayList.Add(fileSize);
                  
//add the fileDetailsArrayList to the global Arraylist
arrayList.Add(fileDetailsArrayList);
}

My question is concerning the line marked with the ***'s. Is this good practice? Every time that chunk of code loops it will create a new ArrayList called fileDetailsArrayList, which works, but something in my mind tells me i'm causing unnessaccary memory usage or something.

If its all good, and C# handles it ok, then thats fine :-)
0
pee_cee
Asked:
pee_cee
  • 3
  • 2
  • 2
  • +2
1 Solution
 
Thandava VallepalliCommented:
No problem.  CLR (Runtime Engine) is will handle it.......
0
 
Jesse HouwingScrum Trainer | Microsoft MVP | ALM Ranger | ConsultantCommented:
It's better to do this:

ArrayList fileDetailsArrayList = null;
FileInfo fi = null;
long fileSize = 0;

foreach(string fileName in fileEntries)
{
fileDetailsArrayList = new ArrayList();
fi = new FileInfo(fileName);
fileSize = fi.Length;
               
//put the name and size into an ArrayList
fileDetailsArrayList.Add(fileName);
fileDetailsArrayList.Add(fileSize);
               
//add the fileDetailsArrayList to the global Arraylist
arrayList.Add(fileDetailsArrayList);
}

As this prevents pointer allocation on each.
0
 
makerpCommented:
The code posted by ToAoM does not provide any significant performance gains, all it does is unneccesarily increase the scope of your variables. Stick with what you have, its more consise.

-P
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
makerpCommented:
>>As this prevents pointer allocation on each.

how is this so, each loop still causes new to be called. its just down to where in the code the variables are declared, in your case outside of the loop instead of inside. in anycase there are still 3 variables declared taking up exactly the same amount of space.
0
 
pee_ceeAuthor Commented:
Ok, so really there was nothing wrong with my code?
If this is the case, i assume itsvtk gets the points?
0
 
Jesse HouwingScrum Trainer | Microsoft MVP | ALM Ranger | ConsultantCommented:
There is nothing wrong with it, but it is common to declare your variables outside of a loop, so that they don't get declared and removed each time round. The performance normally is neglectable, but in some cases it can give you a major increase in speed, especially if the variables internally use a lot of value types.
0
 
Razzie_Commented:
I agree with ToAoM - declare it outside the loop because inside it CAN cause a performance loss.

But what I don't understand is why you would initialize the arraylist inside the loop anyway. Why not just:

ArrayList fileDetailsArrayList = new ArrayList();

foreach(string fileName in fileEntries)
{
             
FileInfo fi = new FileInfo(fileName);
long fileSize = fi.Length;
             
//put the name and size into an ArrayList
fileDetailsArrayList.Add(fileName);
fileDetailsArrayList.Add(fileSize);
             
//add the fileDetailsArrayList to the global Arraylist
arrayList.Add(fileDetailsArrayList);
}


----------------------------

This would be even better.
0
 
pee_ceeAuthor Commented:
That would not work for me, as fileDetailsArrayList needs to be empty each time i enter the loop.
With that suggestion, i believe the fileDetailsArrayList will just grow in size.

I'm not really keen on this implementation anyway, so may be rethinking the design anyway!

I'll give points to ToAoM, but if anyone disagrees, i'm sure an admin can sort it out!
0
 
Razzie_Commented:
Ok then what I don't understand is why you want to create an arraylist only to add 1 long and 1 string to the global arraylist? seems like overkill to me anyway and bad for performance! You should think about using a Hashtable instead and store a filename with its corresponding filesize as a key / value pair.

Regards,

Razzie
0
 
pee_ceeAuthor Commented:
a-ha. sounds like a plan batman. i shall look into this.

basically, i want a list of lots of files, with their sizes.
i remember doing java hashtables many moons ago.....i shall investigate.

thanks for the help all.
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 3
  • 2
  • 2
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now