Convert algorithm from C# to VB.NET fail
I'm trying to convert the following algorithm from C# to VB.NET and the VB.NET I have is not producing the same results as my C# algorithm, can someone tell me where I've gone wrong in my conversion?
public static IEnumerable<T[]> Combinations<T>(this IEnumerable<T> elements, int k)
{
List<T[]> result = new List<T[]>();
// single combination
if (k == 0)
{
result.Add(new T[0]);
}
else
{
int current = 1;
foreach (T element in elements)
{
//combine each element with k-1 combinations of subsequent elements
result.AddRange(elements
.Skip(current++)
.Combinations(k - 1)
.Select(combination => (new T[] { element }).Concat(combination).ToArray())
);
}
}
return result;
}
This is what I've got in VB.NET:
<Extension()>
Public开发者_运维技巧 Function Combinations(Of T)(ByRef elements As IEnumerable(Of T), ByVal k As Integer) As IEnumerable(Of T())
Dim result As New List(Of T())()
'single combination'
If k = 0 Then
result.Add(New T(-1) {})
Else
Dim current As Integer = 0
For Each element As T In elements
'combine each element with k - 1 combinations of subsequent elements'
Dim local As T = element
result.AddRange(elements.Skip(current = current + 1).Combinations(k - 1).Select(Function(combs) (New T() {local}).Concat(combs).ToArray()))
Next
End If
Return result
End Function
Something is wrong, but I'm not sure what, I'm guessing the problem is somewhere in the lambda.
Can anybody point out what I've done wrong with my conversion?
Use a code converter...
<System.Runtime.CompilerServices.Extension> _
Public Shared Function Combinations(Of T)(elements As IEnumerable(Of T), k As Integer) As IEnumerable(Of T())
Dim result As New List(Of T())()
' single combination
If k = 0 Then
result.Add(New T(-1) {})
Else
Dim current As Integer = 1
For Each element As T In elements
'combine each element with k-1 combinations of subsequent elements
result.AddRange(elements.Skip(System.Math.Max(System.Threading.Interlocked.Increment(current),current - 1)).Combinations(k - 1).[Select](Function(combination) (New T() {element}).Concat(combination).ToArray()))
Next
End If
Return result
End Function
I'm no expert on VB at all, but the problem might stem from either:
current = current + 1
is not the same as current++
. It is (basically) the same as ++current
. These have way different behavior.
Since VB doesn't support inline post-increment operators directly, the closest bug-free implementation I can think of is (in C#, since I don't know VB):
int current = 0;
Func<int> getCurrentThenIncrement = () =>
{
int previous = current;
current = current + 1;
return previous;
};
// ...
.Skip(getCurrentThenIncrement())
Or, it could stem from:
Public Function Combinations(Of T)(ByRef elements ...
When I use .Net Reflector to look at it and "convert" it to VB, elements
seems to be ByVal
.
There’s no need for the in-place increment of current at all, as far as I can see. Just increment it after the Linq expression. On the other hand, you should initialise current
with 1
, same as in C#.
Furthermore, your “base case” is a bit weird; you can just write this:
result.Add(New T() { })
No need for the -1
.
I don't have a compiler on hand to check, but I would convert it like this:
<Extension()>
Public Function Combinations(Of T)(elements As IEnumerable(Of T), k As Integer) As IEnumerable(Of T())
Dim result As New List(Of T())()
' single combination
If k = 0 Then
result.Add(New T() {})
Else
Dim current As Integer = 1
For Each element As T In elements
'combine each element with k-1 combinations of subsequent elements
result.AddRange(elements
.Skip(PostfixInc(current))
.Combinations(k - 1)
.Select(Function(combination) (New T() { element }).Concat(combination).ToArray())
)
Next
End If
Return result
End Function
Private Function PostfixInc(ByRef value As Integer) As Integer
Dim currentValue = value
value += 1
Return currentValue
End Function
That's as direct a conversion of each syntax element as I can think of right now. Once you have it working, then think about cleaning it up (e.g. try to remove side effects from the LINQ expression).
EDIT:
Differences from your code:
New T(-1) {}
should beNew T() {}
.current
should be initialised to 1 not 0.Skip(current = current + 1)
will not do what you want. In fact, it does nothing, becausecurrent = current + 1
is a boolean expression that will always evaluate to false. VB.NET will silently convert false to 0 if option strict is off, or throw a compiler error if option strict is on.
精彩评论