VB.Net recursion issue

Question

I had a recursive function in c# that worked well. But when I converted it into VB.Net, it gives out of index error. Strange thing is that, when it is debugged in VB, the code goes to the 'End Sub' line and again goes into the recursive function call. Any idea why this happens?

It is a simple function to correct make ordered age groups from a list of groups. See the code below.

The final expected groups are 0-10, 30-60 and 75-90.

Public Class AgeGroup
    Public Lower As Integer
    Public Higher As Integer
End Class

Public Class RngTest
    Public Sub MakeUniqueRanges(ByVal ranges As List(Of AgeGroup))
        For i As Integer = 0 To ranges.Count - 2
            Dim first As AgeGroup = ranges(i)
            Dim second As AgeGroup = ranges(i + 1)

            If first.Higher > second.Lower Then
                'range is mixed..construct a new range by mixing the two ranges
                Dim newRange As New AgeGroup()
                newRange.Lower = first.Lower
                newRange.Higher = second.Higher
                'delete the existing 2 ranges
                ranges.RemoveAt(i)
                ranges.RemoveAt(i)
                ranges.Insert(i, newRange)

                MakeUniqueRanges(ranges)
            End If
        Next
    End Sub
End Class

Module RngModule
    Sub Main()
        Dim ranges As New List(Of AgeGroup)()
        ranges.Add(New AgeGroup() With {.Lower = 0, .Higher = 10})
        ranges.Add(New AgeGroup() With {.Lower = 30, .Higher = 40})
        ranges.Add(New AgeGroup() With {.Lower = 35, .Higher = 50})
        ranges.Add(New AgeGroup() With {.Lower = 45, .Higher = 60})
        ranges.Add(New AgeGroup() With {.Lower = 75, .Higher = 90})
        Dim test As New RngTest()

        test.MakeUniqueRanges(ranges)
        Console.ReadLine()
    End Sub
End Module

this is the c# method that works well

static void MakeUniqueRanges(List<AgeGroup> ranges)
{
    for (int i = 0; i < ranges.Count - 1; i++)
    {
        AgeGroup first = ranges[i];
        AgeGroup second = ranges[i + 1];

        if (first.Higher > second.Lower)
        {
            //range is mixed..
            AgeGroup newRange = new AgeGroup();
            newRange.Lower = first.Lower;
            newRange.Higher = second.Higher;

            ranges.RemoveAt(i);
            ranges.RemoveAt(i);
            ranges.Insert(i, newRange);
            MakeUniqueRanges(ranges);
        }
    }
}

I wouldn't describe the C# version as one that "works well". The recursion seems wholly unnecessary. — Damien_The_Unbeliever
– Damien_The_Unbeliever, Commented May 2, 2014 at 9:33
@Fabio - you mean in the c# code? no it does not give any error.. — vinayan
– vinayan, Commented May 2, 2014 at 9:50
@MattWilko - c# code was working as intended but not working sensibly enough.. — vinayan
– vinayan, Commented May 2, 2014 at 12:07

Damien_The_Unbeliever · Accepted Answer · 2014-05-02 10:08:11Z

This is the non-recursive C# version:

static void MakeUniqueRanges(List<AgeGroup> ranges)
{
    for (int i = 0; i < ranges.Count - 1; i++)
    {
        AgeGroup first = ranges[i];
        AgeGroup second = ranges[i + 1];

        while (first.Higher > second.Lower)
        {
            //range is mixed..
            AgeGroup newRange = new AgeGroup();
            newRange.Lower = first.Lower;
            newRange.Higher = second.Higher;

            ranges.RemoveAt(i); //Remove "first"
            ranges[i] = newRange; //Replace "second"
            if (i == ranges.Count - 1) return; //Are we now at the end of the list?
            first = ranges[i];
            second = ranges[i + 1];
        }
    }
}

And this should be the VB equivalent:

Public Sub MakeUniqueRanges(ByVal ranges As List(Of AgeGroup))
    Dim i as Integer = 0
    While i <= ranges.Count - 2
        Dim first As AgeGroup = ranges(i)
        Dim second As AgeGroup = ranges(i + 1)

        While first.Higher > second.Lower
            'range is mixed..construct a new range by mixing the two ranges
            Dim newRange As New AgeGroup()
            newRange.Lower = first.Lower
            newRange.Higher = second.Higher

            ranges.RemoveAt(i) 'Remove "first"
            ranges(i) = newRange
            If i = ranges.Count - 1 Then Return

            first = ranges(i)
            second = ranges(i + 1)
        End While
        i = i + 1
    End While
End Sub

So, why does your original attempt not work? Because in VB, the number of iterations to perform is fixed when you first start the loop. The end value isn't reassessed each time through the loop. So this code:

Dim i = 5
For j = 0 To i
    i = 1
    Console.WriteLine("Hello")
Next

Prints Hello 6 times. Whereas in a C# for loop, the termination condition is re-evaluated in full during each time through the loop.

See For...Next:

A While...End While Statement (Visual Basic) or Do...Loop Statement (Visual Basic) works well when you don't know in advance how many times to run the statements in the loop. However, when you expect to run the loop a specific number of times, a For...Next loop is a better choice. You determine the number of iterations when you first enter the loop.

And the reason why the recursion is pointless? Because at any point where this function actually returns, it's already worked it's way along the entire list. What you shouldn't do, having invoked the function recursively, is then continue working through the list again.

And when you make the recursive call, you force it to start again at the start of the list, that your current invocation has already checked.

your method gives me the results 0-10,30-50,45-60,75-90. Actually it should have merged 30-50 and 45-60 to 30-60..i think that is why i tried a recursion..
@vinayan - sorry, I forgot to change the inner If to be a While instead, as I'd done in the C# one that I had tested. Should be correct now.

Collectives™ on Stack Overflow

VB.Net recursion issue

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related