Is there some way to simplify this loop

Question

I have a cycle which goes from -55 to -1, then from 1 to 55 before going back to -55 again. I want to make a loop going from an arbitrary point in this loop to another arbitrary point, but never as far as a full cycle.

Since the end value of the counter might be either a larger or smaller than the start value, I initially made the exit condition for the loop that fra equals til.

However, I also want to do something when fra = til, so in the end my loop ended up looking like what you can see below, with a helper-variable triggering the exit.

I am not all that happy with how this ended up looking though, feeling the logic is a bit clunky. Is there some way to improve the code below?

Option Explicit
Dim første_økt_anodeskift(1 To 10, 1 To 2) As Long
Const farge_anodeskift_1 As Long = 14395790 ' RGB(142, 169, 219)
Sub main()
 Dim syklus As Long
 
 syklus = 1
 
 Call legg_inn_verdier
 Call fargelegg(første_økt_anodeskift(syklus, 1), første_økt_anodeskift(syklus, 2), farge_anodeskift_1)
End Sub
Private Sub fargelegg(fra, til, farge)
 Dim exit_on_next As Boolean
 
 exit_on_next = False
 
 Do
 ' Do stuff
 
 If exit_on_next Then Exit Do
 
 fra = fra + 1
 
 If fra = 0 Then
 fra = 1
 ElseIf fra = 56 Then
 fra = -55
 End If
 
 If fra = til Then exit_on_next = True
 Loop While True
End Sub
Private Sub legg_inn_verdier()
 første_økt_anodeskift(1, 1) = -11: første_økt_anodeskift(1, 2) = 6
End Sub
```

• TinMan: Will the initial value of fra ever be positive?

Answer 1

Maybe this looks better to you. Encapsulating a loop's contained logic in dedicated single-purpose procedures generally makes a loop easier to read and understand(IMO). In this case, the loop conditions requiring the repeated code to run 'one last time' becomes a little more clear as well.

 Option Explicit
 Dim første_økt_anodeskift(1 To 10, 1 To 2) As Long
 Const farge_anodeskift_1 As Long = 14395790 ' RGB(142, 169, 219)
 Sub main()
 Dim syklus As Long
 
 syklus = 1
 
 Call legg_inn_verdier
 Call fargelegg(første_økt_anodeskift(syklus, 1), første_økt_anodeskift(syklus, 2), farge_anodeskift_1)
 End Sub
 Private Sub fargelegg(fra, til, farge)
 Do
 DoStuff fra, til, farge
 
 fra = GetNextFra(fra)
 
 If fra = til Then
 DoStuff fra, til, farge
 Exit Do
 End If
 
 Loop While True
 End Sub
 Private Sub legg_inn_verdier()
 første_økt_anodeskift(1, 1) = -11: første_økt_anodeskift(1, 2) = 6
 End Sub
 Private Sub DoStuff(fra, til, farge)
 ' Do stuff
 End Sub
 Private Function GetNextFra(fra) As Variant
 fra = fra + 1
 
 If fra = 0 Then
 fra = 1
 ElseIf fra = 56 Then
 fra = -55
 End If
 
 GetNextFra = fra
 End Function

• FreeMan: There's no need for Call
• eirikdaude: Yeah, I like this approach, thanks for the input :-)

Answer 2

Without any sample data it is hard to give an exact answer. But I'll give it ago.

This is the Cycle:

• -55 to -1
• 1 to 55
• -55 to -1

Basically, the Cycle is 3 loops with variable start and end values. We can write this problem with an outer loop of 1 to 3 and an inner loop. The trick is to manipulate the start and end values based on the Op's specifiations.

You'll notice that the lowest absolute bounds is 1 and the highest absolute bounds is 55 for all three loops. O don't think the their needs to be any clarification for 1 or negative 1 but naming 55 would be be helpful.

The first thing that I did was replicate the cycle using the loops.

Const LoopSize As Long = 55
For a = 1 To 3
 For b = IIf(a = 2, 1, -LoopSize) To IIf(a = 2, LoopSize, -1)
 
 Next
Next

Now that I reproduced a full cycle it was time to determine the effect that the other conditions will have on the starting and ending values of the inner loop.

I want to make a loop going from an arbitrary point in this loop to another arbitrary point

So it goes from the first point to an end point then exits. fra is initial starting value. What are the possible starting values for fra? They be any int in the first two loops of the cycle but not the third loop. Because the third loop has the same range of numbers as the first loop, there is no value that would skip to the third loop.

If A = 3 then Loop B = -55 to 1

Negative Starting Point (SP)

1. If Loop A = 1 Loop B = SP to -1
2. If Loop A = 2 then Loop B = 1 to 55

Positive Starting Point (SP)

3. Loop A = 2 to 3
4. If Loop A = 2 then Loop B = SP to 55

For positive initial value, we can still Loop a from 1 to 3 because the positive value we cause the first loop to exit without executing.

This logic can be simplified using the VBA Switch statement.

Switch(a = 1, initialValue, a = 2, IIf(initialValue > 0, initialValue, 1), True, -LoopSize)

Refactored Code

Sub Test()
 Fargelegg -55, 10, False
 Fargelegg -25, 10, False
 Fargelegg 5, -15, False
 Fargelegg 25, -25, False
End Sub
Sub Fargelegg(fra, til, farge)
 Const LoopSize As Long = 55
 Dim a As Long
 Dim b As Long
 Dim initialValue As Long
 initialValue = fra
 
 Debug.Print String(2, vbNewLine), "fra: "; fra; " til: "; til
 For a = 1 To 3
 For fra = Switch(a = 1, initialValue, a = 2, IIf(initialValue > 0, initialValue, 1), True, -55) To IIf(a = 2, LoopSize, -1)
 ' Do Something
 Debug.Print fra; ",";
 If fra = til Then Exit For
 Next
 If fra = til Then Exit For
 Next
 
End Sub

Results

 fra: -25 til: 10 
-25 ,-24 ,-23 ,-22 ,-21 ,-20 ,-19 ,-18 ,-17 ,-16 ,-15 ,-14 ,-13 ,-12 ,-11 ,-10 ,-9 ,-8 ,-7 ,-6 ,-5 ,-4 ,-3 ,-2 ,-1 , 1 , 2 , 3 , 4 , 5 , 6 , 7 , 8 , 9 , 10 ,
 fra: 5 til: -15 
 5 , 6 , 7 , 8 , 9 , 10 , 11 , 12 , 13 , 14 , 15 , 16 , 17 , 18 , 19 , 20 , 21 , 22 , 23 , 24 , 25 , 26 , 27 , 28 , 29 , 30 , 31 , 32 , 33 , 34 , 35 , 36 , 37 , 38 , 39 , 40 , 41 , 42 , 43 , 44 , 45 , 46 , 47 , 48 , 49 , 50 , 51 , 52 , 53 , 54 , 55 ,-55 ,-54 ,-53 ,-52 ,-51 ,-50 ,-49 ,-48 ,-47 ,-46 ,-45 ,-44 ,-43 ,-42 ,-41 ,-40 ,-39 ,-38 ,-37 ,-36 ,-35 ,-34 ,-33 ,-32 ,-31 ,-30 ,-29 ,-28 ,-27 ,-26 ,-25 ,-24 ,-23 ,-22 ,-21 ,-20 ,-19 ,-18 ,-17 ,-16 ,-15 ,
 fra: 25 til: -25 
 25 , 26 , 27 , 28 , 29 , 30 , 31 , 32 , 33 , 34 , 35 , 36 , 37 , 38 , 39 , 40 , 41 , 42 , 43 , 44 , 45 , 46 , 47 , 48 , 49 , 50 , 51 , 52 , 53 , 54 , 55 ,-55 ,-54 ,-53 ,-52 ,-51 ,-50 ,-49 ,-48 ,-47 ,-46 ,-45 ,-44 ,-43 ,-42 ,-41 ,-40 ,-39 ,-38 ,-37 ,-36 ,-35 ,-34 ,-33 ,-32 ,-31 ,-30 ,-29 ,-28 ,-27 ,-26 ,-25 ,

• eirikdaude: Oh, that's a really clever solution! Thanks a lot for your input!
• eirikdaude: And I learned about iif too, that's a really handy function - I'll have to remember it for the future :-)

Answer 3

If I may give you some additional advice. I noticed your identifiers seem to be in Norwegian (?). It is usually recommended to use plain ASCII and even names in English.

The problem with identifiers: some compilers or interpreters may have problems with non-standard characters. The behavior is not always documented clearly and may be unpredictable. Norwegian (or Danish...) should be OK but I am not sure VBA still supports the full Unicode range. In the context of VBA and Windows the reference is probably the Windows-1252 character set, which should cover most Latin alphabets.

But if you used variables names in Arabic or Japanese script, I am not sure how the program will react.

Now the problem with using variable names in your native language is that foreign people will have more difficulty to grasp the logic of your code, because the names are not intuitive to them.

More and more projects are teamwork, they may also evolve into open-source projects and be published on Github. If you are not using English you are limiting the audience of possible adopters and contributors.

Also, variables that have suffixes like _1 or _2 are poor names. It is a telltale sign that the naming conventions need improvement.

The second thing I noticed is the lack of comments. This is typical VB spaghetti code, it is not easy to follow the stream of execution because of:

• structure, plus calling routines that have loops
• language (the choice of identifiers)
• lack of comments

All these factors compounded, result in code that is difficult to comprehend for outsiders like us.

Comments are very important, not just for reviewers like us but also for colleagues or other people who may have to tweak your code while you are on holiday or after you have left the company. Most importantly, the comments are also for you, because in 6 months you will have forgotten your train of thought, you will have to re-analyze tour own code and wtf you were thinking.

Simply put, comments makes the purpose of your code more clear.

Also, you should avoid multi-line statements like this:

første_økt_anodeskift(1, 1) = -11: første_økt_anodeskift(1, 2) = 6

Just have two lines, it's more readable (in spite of not making the intent more clear):

Private Sub legg_inn_verdier()
 første_økt_anodeskift(1, 1) = -11
 første_økt_anodeskift(1, 2) = 6
End Sub

But notice how the statements are neatly aligned.

In terms of performance and clarity: choose the right data type for variables and return values. That means do not use the Long datatype for short numbers. See this post for more discussion on VBA data types.

The function GetNextFra returns an integer value, so do not declare the function as Variant. Ever. Integer (or Long as per post quoted above) should do.

This function can be simplified (and it should). If you have a cycle that you want to reset at 55, what you need is a modulus. The implementation would be like this (untested):

Private Function GetNextFra(fra) As Integer
 GetNextFra = (fra mod 55) +1
End Function

What this function does: return fra + 1, unless fra = 55, then we reset the counter to 1. If this is not exactly what you want, you should be able to make adjustments accordingly. Voilà.

But since this function is now a one-liner, you can as well ditch the function. Copy the one-liner to your program body. Done. You have already simplified code.

To be honest I don't really understand what your code actually does. If I did then I could maybe suggest some more ideas.

• eirikdaude: Thanks for the input. With regards to using longs instead of ints, all ints are converted to longs anyway, so by using longs in the first place you avoid that conversion.
• Greedo: Best answer IMO on the topic of Int vs Long (full disclosure, I have made some minor edits to that in the past). Bottom line; memory-wise Integer beats Long, so a large array of Integers is a good idea. Performance though is generally about speed - Longs are faster than Integers as they avoid the implicit 32-bit conversion to fit in the hardware registers which is an extra machine instruction. So recommending "do not use the Long datatype for short numbers" on the basis of performance could be misleading if we expect performance to be speed.
• Greedo: Either memory-performance or execution-time-performance will be affected negligibly by this, so clarity is much more important. In which case consistency is king, and consistent use of Longs unless an Integer is really needed (see the link for when) is the best way to go I think. I agree with you that Variant is rarely the best choice, as then you might miss type-conversion errors and introduce bugs. The point is I think performance of the different integer holding datatypes is pretty irrelevant in code like this, maybe reword that bit?