Strange problem with .Net inheritance and member visibility

Question

I have a problem in a VB.Net class library which I have simplified greatly down to the following...

Public MustInherit Class TargetBase

End Class

Public Class TargetOne
    Inherits TargetBase
End Class

Public Class TargetTwo
    Inherits TargetBase
End Class

Public Class TargetManager
    Public Sub UpdateTargets(ByVal Targets As List(Of TargetBase))
        For Each objTarget As TargetBase In Targets
            UpdateTarget(objTarget)
        Next
    End Sub

    Private Sub UpdateTarget(ByVal Value As TargetOne)

    End Sub

    Private Sub UpdateTarget(ByVal Value As TargetTwo)

    End Sub
End Class

This will not compile due to a syntax error on the UpdateTarget(objTarget) line - Overload resolution failed because no accessible 'UpdateTarget' can be called without a narrowing conversion

So I changed the For-Each loop to use Object instead of TargetBase...

For Each objTarget As Object In Targets
    UpdateTarget(objTarget)
Next

This now compiles but I get a run-time error - Public member 'UpdateTarget' on type 'TargetManager' not found.

So I took the obvious next step of making the 2 UpdateTarget() overloads Public (instead of Private).

Public Sub UpdateTarget(ByVal Value As TargetOne)

End Sub

Public Sub UpdateTarget(ByVal Value As TargetTwo)

End Sub

This now works!

I can just about understand why changing it to Object would work but why must these methods be made Public when I am only calling them from within the same class - I would rather they weren't available outside this class.

Can anybody explain this?

Thanks in advance (and sorry for the length of this question!)

Additional Thanks for everyones answers so far. Ive got the workaround (make UpdateTarget methods Public) that makes it work. Another workaround would be to do a TypeOf check on objTarget and then DirectCast before calling UpdateTarget, like...

For Each objTarget As Object In Targets
    If TypeOf objTarget Is TargetOne Then
        UpdateTarget(DirectCast(objTarget, TargetOne))
    ElseIf TypeOf objTarget Is TargetTwo Then
        UpdateTarget(DirectCast(objTarget, TargetTwo))
    End If
Next

This will also work - I posted the question because I really wanted to understand why changing visibility of UpdateTarget from Private to Public got rid of the run-time error, totally against my understanding!

Answer 1

As Adam said, the compiler doesn't know which method it should be calling. However, this looks like the UpdateTarget method should be an instance method that each Target type overrides. This way you can iterate over the list and simply call UpdateTarget on objTarget.

The other advantage of this is you are better encapsulating the code. TargetManager doesn't need to know what the update actually does, just that it has to be called. Furthermore, down the road when you write a TargetThree, you don't have to change TargetManager in order to be able to update the the new type.

Answer 2

Update: Since you indicated in a comment that the normal polymorphic approach to this problem isn't possible in your case, I'd at least strongly advise changing your TargetManager class to have a single UpdateTarget method accepting a TargetBase parameter. Then check for the appropriate type within that method. This prevents the potential problem of having...

If TypeOf x Is A Then
    DoSomething(DirectCast(x, A))
ElseIf TypeOf x Is B Then
    DoSomething(DirectCast(x, B))
End If

...all over the place.

In other words, put that ugly check in one place:

Public Class TargetManager
    Public Sub UpdateTarget(ByVal target As TargetBase)
        Dim t1 = TryCast(target, TargetOne)
        If t1 IsNot Nothing Then
            UpdateTargetOne(t1)
            Return
        End If

        Dim t2 = TryCast(target, TargetTwo)
        If t2 IsNot Nothing Then
            UpdateTargetTwo(t2)
            Return
        End If
    End Sub

    ' I would also recommend changing the targets parameter type here '
    ' to IEnumerable(Of TargetBase), as that is all you need to do '
    ' a For Each loop. '
    Public Sub UpdateTargets(ByVal targets As IEnumerable(Of TargetBase))
        For Each objTarget As TargetBase In Targets
            UpdateTarget(objTarget)
        Next
    End Sub

    Private Sub UpdateTargetOne(ByVal target As TargetOne)
        ' Do something. '
    End Sub

    Private Sub UpdateTargetTwo(ByVal target As TargetTwo)
        ' Do something. '
    End Sub
End Class

---

You've got polymorphism backwards.

Right off the bat, this is the way I intuitively think you really want this to work:

Public MustInherit Class TargetBase
    Protected Friend MustOverride Sub Update()
End Class

Public Class TargetOne
    Inherits TargetBase

    Protected Friend Overrides Sub Update()
    End Sub
End Class

Public Class TargetTwo
    Inherits TargetBase

    Protected Friend Overrides Sub Update()
    End Sub
End Class

Public Class TargetManager
    Public Sub UpdateTargets(ByVal Targets As List(Of TargetBase))
        For Each objTarget As TargetBase In Targets
            objTarget.Update()
        Next
    End Sub
End Class

Answer 3

If you're using VS2008 or 2010 you can use the ExtensionAttribute to do what you want without casting out.

Answer 4

Though VB is not my specialty, I think I can answer your questions correctly.

In the first version of your program you have UpdateTarget(objTarget) where objTarget is of type TargetBase. VB reasons as follows:

• The receiver of the call is "Me". That has a well-known compile-time type, TargetManager.
• The argument of the call is "objTarget". That has a well-known compile-time type, TargetBase.
• Since the receiver and the arguments all have types, we should do overload resolution to determine which version of UpdateTarget to call.
• Overload resolution determines that both versions of UpdateTarget require a potentially unsafe conversion from TargetBase to a more specific type.
• Therefore overload resolution fails.

In the second version objTarget is of type Object. VB reasons as follows.

• The argument of the call is of type Object.
• Again, overload resolution gives us nothing good here.
• Since overload resolution failed and something is of the Object type, and Option Strict is not on, generate code that does the analysis again at runtime using the runtime type of the argument.
• Late-bound analysis requires that the method called be public. Why? Because suppose this code were not inside TargetManager. Would you want to be able to call the private method from outside TargetManager via late binding but not via early binding? Probably not. That seems dangerous and wrong. Unfortunately, VB's late binding does not distinguish between late binding done inside TargetManager and late binding done outside. It simply requires that the methods be public in order to be called late bound.

In the third version, we go to late binding and the late binding succeeds at runtime.

What I would do here is:

1. Turn option strict on.

2. do TWO loops. Yes, that's not as efficient because you do the loop twice, but, big deal. If this isn't the slowest thing in your program then who cares if it gets a few milliseconds slower.

I don't know exactly what the VB syntax is, but in C# I would write this like this:

public void UpdateTargets(IEnumerable<TargetBase> targets) 
{
    foreach(var targetOne in targets.OfType<TargetOne>())
        UpdateTarget(targetOne);
    foreach(var targetTwo in targets.OfType<TargetTwo>())
        UpdateTarget(targetTwo);
}

Nice and simple. Go through the collection twice, first taking out the TargetOnes, then taking out the TargetTwos.

(Note also that if I'm not using any feature of List, then I'd make the argument IEnumerable instead, so that the method becomes more general.)