Link to home
Create AccountLog in
Avatar of pee_cee
pee_cee

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 :-)
Avatar of Thandava Vallepalli
Thandava Vallepalli
Flag of United States of America image

No problem.  CLR (Runtime Engine) is will handle it.......
ASKER CERTIFIED SOLUTION
Avatar of Jesse Houwing
Jesse Houwing
Flag of Netherlands image

Link to home
membership
Create a free account to see this answer
Signing up is free and takes 30 seconds. No credit card required.
See answer
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
>>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.
Avatar of pee_cee
pee_cee

ASKER

Ok, so really there was nothing wrong with my code?
If this is the case, i assume itsvtk gets the points?
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.
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.
Avatar of 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!
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
Avatar of 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.