0

I have 3 worksheets. The first 2 have i convert into two arrays (array1 & array2), and then do calculations between them to create the third.

The Macro I created utilizes this piece of code ---

Z = 1

For x = 1 To UBound(array1, 1)
    For y = 1 To UBound(array2, 1)
        If array1(x, 4) = 0 Then
            GoTo Line1
        End If
        If array1(x, 1) = array2(y, 1) And array1(x, 2) = array2(y, 3)Then
            If array1(x, 4) > array2(y, 5) Then
                array3(z, 1) = array1(x, 3)
            ElseIf array1(x, 4) = array2(y, 5) Or array1(x, 4) < array2(y, 5) Then
                array3(z, 1) = array1(x, 3)
            End If
            z = z + 1
        End If
    Next y
Line1:
Next x

It takes a piece of array1 and loops it through array2 and creates a result in array3

Basically when array1(x, 4) = 0, I need it to move on to the next X. I can't figure out how to loop this without the GoTo Line1.

If i move it down, then it will continue to loop through array2(y), instead of moving on to next X. If i move it above, then y resets and it runs through the For y loop again

Also is using GoTo Line X, bad practice in VBA. Should I always try to avoid using it. I am fairly new to it.

Tomalak
  • 332,285
  • 67
  • 532
  • 628
UserX
  • 157
  • 13
  • 1
    goto can be useful but it is also insanely powerful, so best to make sure you understand everything before you simply just stick it in code. but in your case ou might need to use it – Doug Coats Feb 18 '18 at 21:27
  • Please indent your code properly and remove the empty lines. Goto is in general never necessary (except for the `On Error Goto ...` statements). Avoid it in regular code. – Tomalak Feb 18 '18 at 21:30
  • @Tomalak is it necessary for this code? I am not sure of another way to format this – UserX Feb 18 '18 at 21:38
  • 1
    Test for the opposite and remove the go-to and put the code in side the if. – Scott Craner Feb 18 '18 at 21:41
  • @UserX As I said, `GoTo` is never necessary. Using it increases the probability of bugs in your code and the general chance of headaches. – Tomalak Feb 18 '18 at 21:50
  • Right... I cant figure out a way to do it with out it. @ScottCraner I think you mean move the goto if before the For y line and change it to Not = to 0? When I do that, it causes the x to continue to cycle through the y loop and does not reach the If array3(x, 4) = 0. Because of this I get a bunch of 0s in my array because it maches the logic, but it returns a 0 because that is what is left in the array3(x, 4) – UserX Feb 18 '18 at 21:54
  • Why don’t you just use the code in [this answer](https://stackoverflow.com/questions/48847738/setting-unknown-array-boundaries-and-loop/48854307#48854307)? – DisplayName Feb 18 '18 at 21:57
  • @DisplayName yea I tried that. I think i wasn't correctly formatting because the answer was a general form of the code and not specific, as I asked the question generally. I could get the redim to work but not the loop to break where i needed it to. A combination of both of these seems to be the best solution – UserX Feb 18 '18 at 22:07

1 Answers1

1

Writing code that relies on GoTo is widely considered bad style.

VB(A) has a few built-in constructs that are used for its way of error handling that need GoTo. Those are unavoidable. All the other ones should be avoided.

In this case it's fairly easy, you can break a For loop with Exit For:

Z = 1

For x = 1 To UBound(array1, 1)
    For y = 1 To UBound(array2, 1)
        If array1(x, 4) = 0 Then Exit For
        If And array1(x, 1) = array2(y, 1) And array1(x, 2) = array2(y, 3) Then
            If array1(x, 4) > array2(y, 5) Then
                array3(z, 1) = array1(x, 3)
            ElseIf array1(x, 4) = array2(y, 5) Or array1(x, 4) < array2(y, 5) Then
                array3(z, 1) = array1(x, 3)
            End If
            z = z + 1            
        End If
    Next y
Next x

Alternative (has one nesting level more):

For x = 1 To UBound(array1, 1)
    If array1(x, 4) <> 0 Then
        For y = 1 To UBound(array2, 1)
            If And array1(x, 1) = array2(y, 1) And array1(x, 2) = array2(y, 3) Then
                If array1(x, 4) > array2(y, 5) Then
                    array3(z, 1) = array1(x, 3)
                ElseIf array1(x, 4) = array2(y, 5) Or array1(x, 4) < array2(y, 5) Then
                    array3(z, 1) = array1(x, 3)
                End If
                z = z + 1            
            End If
        End If
    Next y
Next x
Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • thank @tomalak. I was unaware of the exit for command. Would that take it to the line directly after the Next Y? – UserX Feb 18 '18 at 22:05
  • @UserX, yes it would. Be aware that `Exit For` only exists one level of `For` loop. – 41686d6564 stands w. Palestine Feb 18 '18 at 22:38
  • @tomalak, it's worth mentioning that `GoTo` isn't ALWAYS bad. Yes, it's probably bad for 99% of the cases, but it can be useful sometimes *if **only** used to jump forward*. An example to that would be [breaking nested loops](https://stackoverflow.com/a/5312563/4934172). And yes, an alternative would be to use a separate function, but with multiple local variables, one would end up with way more complicated code, while using `GoTo` in such case wouldn't cause any harm. – 41686d6564 stands w. Palestine Feb 18 '18 at 22:38
  • 1
    When you are good enough to recognize the 1% of use cases (it's probably way less than that, really) where `GoTo` might be justified to use, then you don't need to ask about it. As long as you have to ask, you are not good enough and should never use it, very simple. Personally, I have not yet found even a single valid use case that did not have a better alternative through refactoring. – Tomalak Feb 18 '18 at 22:50