C# foreach memory cache and string memory optimization

Question

foreach (string sn in MACOrSerial.Split(','))
{
    MACOrSerial = sn.Trim();   
}

MACORSerial contains a string of text (EX. AA123241, BB123431, CC1231243) separated by commas. I grab one substring and place that into the same MACORSerial.

This will not cause me any problems as the the foreach still will be using the original MACOrSerial in memory.

While I think this is the most memory efficient approach, is it proper or should I just make another string with a new name such as

MacORSerialSubString = sn.Trim()?

I am not having any memory issues. I just want to make sure my code is clean and concise.

Are you having an actual memory issue with this that you are trying to solve? — Oded, Mar 22 '12 at 14:49
Oded is right. Spend your time solving real problems that are actually causing customer pain. Write your code so that every variable has a clearly understandable purpose, not so that you somehow "optimize" your use of memory. — Eric Lippert, Mar 22 '12 at 14:51
I had to read through this code about 5 times before I could grok what was happening. It's unreadable and likely doesn't address any of the perceived memory issues you're having. — RQDQ, Mar 22 '12 at 14:52
@HarHaHu - I think you can see by the tenor of the responses that the above approach is neither intuitive or clean. — RQDQ, Mar 22 '12 at 14:54
Then (1) use one variable to mean *exactly one thing*, and (2) name your variables as *nouns*. *MACOrSerial* -- two adjectives -- should probably be something like "addressList". The inner variable should be something like "trimmedAddress". — Eric Lippert, Mar 22 '12 at 14:56

Oded · Accepted Answer · 2012-03-22T14:57:38.700

8

Your assumption is incorrect - the loop goes over the string[] resulting from the Split - these are all new string instances.

You are not saving any memory by reassigning a string to the original variable and you are losing readability by reuse of a variable.

Here is one approach that is more readable and uses some of the built in capabilities:

string[] serialNumbers = MACOrSerial.Split(new [] {',', ' ', '\r', '\n' }, 
                                   StringSplitOptions.RemoveEmptyEntries);

foreach (string sn in serialNumbers)
{
    // do stuff
}

edited Mar 22 '12 at 14:57

answered Mar 22 '12 at 14:50

Oded

489,969
99
883
1,009

+1 thanks, you sure not even .00001 seconds?!? for a new string assignment? – Har Mar 22 '12 at 14:57
@HarHaHu - Unless you are working on many millions of records, this is not likely to be of any significance. – Oded Mar 22 '12 at 14:58
fantastic and comprehensive answer! Thanks. – Har Mar 22 '12 at 15:03
3

@HarHaHu: If you have a performance concern, **measure it** and then you'll know. First, set a performance goal so that you know what acceptable and unacceptable performance is. Then measure your program against the goal. If it is acceptable **do not fix it; it is not broken.** If it is unacceptable then *get a profiler*, *run the profiler*, *find the slowest thing* and *fix that*. The few microseconds it takes to allocate this string is unlikely to be the real problem, so *stop wasting time thinking about how to fix non-problems.* Spend time tracking down and fixing *real* problems. – Eric Lippert Mar 22 '12 at 16:00

Chris Shain · Answer 2 · 2012-03-22T14:57:46.643

4

My $.02- the code as you posted is extremely un-intuitive, because it breaks with the expected pattern.

IMHO it is more important to write the code in a way that other developers can understand at a glance than it is to (actually / attempt to) micro-optimize for memory, especially when (as others have pointed out) your micro-optimization does not actually reduce the amount of memory used.

edited Mar 22 '12 at 14:57

answered Mar 22 '12 at 14:50

Chris Shain

50,833
6
93
125

Sure. Not to plug my own answer, but you may also want to read what's become a somewhat good reference on how .NET treats strings here: http://stackoverflow.com/questions/9132338/how-many-string-objects-will-be-created-when-using-a-plus-sign/9132374#9132374, especially the bit at the end about concatenation loops. – Chris Shain Mar 22 '12 at 15:00

score 2 · Answer 3 · answered Mar 22 '12 at 14:49

2

I would create a new variable so that your code is more clear. It doesn't have any noticable impact on memory.

answered Mar 22 '12 at 14:49

Daniel A. White

187,200
47
362
445

score 1 · Answer 4 · answered Mar 22 '12 at 14:51

For clarity (and later maintaining) of the code, I would prefer use of a separate variable. Plus I would remove the empty strings up front:

foreach (string sn in MACOrSerial.Split(',', StringSplitOptions.RemoveEmptyEntries))
{
    string MacORSerialSubString = sn.Trim();
}

score 1 · Answer 5 · answered Mar 22 '12 at 14:53

In reality, your iteration will be using the result of the MACOrSerial.Split() call, which is an array that is independent from the MACOrSerial variable.

There will be no difference between using MACOrSerial or another string variable in terms of memory usage, each time sn.Trim() is called, a new string is generated and it's just the reference to this new string that gets placed in your string variable.

C# foreach memory cache and string memory optimization

5 Answers5