Why is the app so slow

BadKarma

I am forced to solve a bug in our VB application, the original programmer has left the building a long time ago. Due to the long and complex function they have given the variable a simple name.

Dim nFoo() As Integer
Dim sfoo() As String
Dim sFoo1() As String

So i went on looking what could be placed inside a nFoo, and i stumbled on this gem.

j = 0
For i = 1 To lvwCardGrp.ListItems.Count
  ReDim Preserve nFoo(j)
  nFoo(j) = Mid$(lvwCardGrp.ListItems(i).Key, 3)
  j = j + 1
Next

And of course why change a winning strategy the same for the sfoo's.

Learn from the mistakes of others, you may not live long enough to make them all yourself.

Paul Conrad

Nice gem :laugh:

"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham

Graham Bradshaw

In the original coders defense, if there (previously) was a condition around the ReDim / nFoo = / j = lines, the logic makes perfect sense.

BadKarma

Graham Bradshaw wrote:

In the original coders defense, if there (previously) was a condition around the ReDim / nFoo = / j = lines, the logic makes perfect sense.

Actually in one of the three cases this was true, but then one could do it like this (which is what i chose)

j = 0
ReDim nFoo(lvwCardGrp.ListItems.Count - 1)
For i = 1 To lvwCardGrp.ListItems.Count
  if some_condition then
    nFoo(j) = Mid$(lvwCardGrp.ListItems(i).Key, 3)
    j = j + 1
  endif
Next
ReDim nFoo(j - 1)

In this case only one allocation happens. And in the end free up the unnecessary space, without the preserve statement which needs to copy the data to a new location.

Learn from the mistakes of others, you may not live long enough to make them all yourself.

PIEBALDconsult

I don't see how. Unless I read it wrong, j is unnecessary, the array should be redimed before the loop, we already know how many items it needs to hold.

Pete OHanlon

That's just too bad. Way too bad.

Deja View - the feeling that you've seen this post before.

My blog | My articles

Graham Bradshaw

PIEBALDconsult wrote:

we already know how many items it needs to hold.

Not if you only add items to the array in some condition, which was the point I was trying to make. See the other reply to my post for an example.

Graham Bradshaw

Shouldn't ReDim nFoo(j - 1) be ReDim Preserve nFoo(j - 1)? As the code is, you reinitialise nFoo.

PIEBALDconsult

Ah, I see, I did read it wrong.

BadKarma

Graham Bradshaw wrote:

Shouldn't ReDim nFoo(j - 1) be ReDim Preserve nFoo(j - 1)? As the code is, you reinitialise nFoo.

Yes, you are correct. Thanks for pointing it out, must have slipped through my fingers while typing it over from the real code. :-O Note to myself: Learn to use Copy-Past ;)

Learn from the mistakes of others, you may not live long enough to make them all yourself.