asked on

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 :-)

Thandava Vallepalli

No problem. CLR (Runtime Engine) is will handle it.......

ASKER CERTIFIED SOLUTION

Jesse Houwing

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

Paul Maker

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

Paul Maker

>>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.

pee_cee

ASKER

Ok, so really there was nothing wrong with my code?
If this is the case, i assume itsvtk gets the points?

Jesse Houwing

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.

Razzie_

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.

pee_cee

ASKER

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!

Razzie_

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

pee_cee

ASKER

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.