3
\$\begingroup\$

Requirements:

  • To store strings into variables, check if they are valid for the field.
  • To be able to easily unit test the validity checks with different parameters.
  • To throw exception when a check fails
  • To scale appropiately for dozens of field types
  • To have a good API for users.
  • To be expandable.

I have the following code. A class that will hold a fund

Public Class FundElement
    Public Name As New FundDetail
    Public Price As New FundDetail
    Public Code As New CodeDetail
End Class

A base class for each individual data field.

Public Class FundDetail
    Protected _value As String

    Public Property value As String
        Get
            Return _value
        End Get

        Set(valueToAssign As String)
            If isValid(valueToAssign) Then
                _value = valueToAssign
            Else
                Throw New ArgumentException
            End If
        End Set
    End Property

    Shared Function isValid(detail As String) As Boolean
        Return True
    End Function
End Class

And some inherited classes that will hold logic specific to certain fields. This is the reason I wanted a class for rather than just a string for each field. It inherits from FundDetail, but it could have been an interface. I chose inheritance so I could have default implementations.

Public Class CodeDetail
    Inherits FundDetail

    Public Shared Shadows Function isValid(value As String) As Boolean
        Dim intPattern As Regex = New Regex("[a-zA-Z]{3}[0-9]{12}")
        Return intPattern.IsMatch(value)
    End Function
End Class

Now, my problem is that the API for my class is as follows

Dim fund as new FundElement

FundElement.Name.Value = "Google Inc"
FundElement.Code.Value = "USD451234151234"

Which means that coders have access to the NAV element, which I want to hide completely. And the API is more verbose than it needs to be. I want the interface to be as follows

FundElement.Name = "Google Inc"
FundElement.Price = "USD451234151234"

I can implement this by changing the class so that each fundDetail instance is private, and by adding a new property as follows

Public Class FundElement

    Private _Name As New FundDetail
    Public Property Name As String
        Set(valueToAssign As String)
            _Name.value = valueToAssign
        End Set
        Get
            Return _Name.value
        End Get
    End Property

    Private _Code As New FundDetails
    Public Property _Code As String
        Set(valueToAssign As String)
            _Code.value = valueToAssign
        End Set
        Get
            Return _Code.value
        End Get
    End Property

    Private _Price As New PriceDetail
    Public Property _Price As String
        Set(valueToAssign As String)
            _Price.value = valueToAssign
        End Set
        Get
            Return _Price.value
        End Get
    End Property

End Class

Now this ended up very verbose for just 3 members. Additionally, the code for each property was almost exactly the same. It's tedious to type and cluttery to read.

Is there any way I can achieve this without having to type the properties every time I want to add a new member? I feel like I should take a more simple approach. But every simple approach comes with a deficiency I have trouble overcoming.

\$\endgroup\$
  • \$\begingroup\$ This is very similar to Validation that also returns error messages. Take a look at my answer or the OP's comment. \$\endgroup\$ – t3chb0t Aug 4 '16 at 9:20
  • \$\begingroup\$ Will your unit test check if CodeDetail works or if Code has the proper business rule attached to it? \$\endgroup\$ – the_lotus Aug 12 '16 at 14:24
  • \$\begingroup\$ @the_lotus the unit tests check if given certain inputs it produces certain results (passes or fails check) as per business requirements. Currently I'm testing isValid directly, which is no longer shared. Just an overriden method. \$\endgroup\$ – Tomas Zubiri Aug 12 '16 at 15:00
  • \$\begingroup\$ @TomasZubiri it's a bit vague but it seems like you are just testing the rule and not testing if the actual business object works properly. I say this because I'm used to working with CSLA. I find it unusual to have your variable set up like this instead of having Name and Code as just string and add the business rule as attributes. \$\endgroup\$ – the_lotus Aug 12 '16 at 15:04
1
\$\begingroup\$

I'm not really a VB.Net'er but you appear to be able to achieve what you're after using the Widening Operator. It allows you to make your value property a Friend so that it can only be accessed from within your module. However, it's not ideal, as you have to create the widening operator in your FundDetail class and any inheritted classes. Depending on the number of different field types you may end up with more typing. I found that I had to change the isValid method to Overridable, rather than Shared in order to get it to be called on derived classes, but that might be because of my lack of VB.Net experience. It also works by creating new instances of objects so there is some overhead. An example of what it looks like is below:

Public Class FundElement
    Public Name As New FundDetail
    Public Price As New FundDetail
    Public Code As New CodeDetail
End Class

Public Class FundDetail

    Protected _value As String

    Friend Property value As String
        Get
            Return _value
        End Get

        Set(valueToAssign As String)
            If isValid(valueToAssign) Then
                _value = valueToAssign
            Else
                Throw New ArgumentException
            End If
        End Set
    End Property

    Overridable Function isValid(detail As String) As Boolean
        Return True
    End Function

    Public Shared Widening Operator CType(ByVal val As String) As FundDetail
        Dim retVal As New FundDetail
        retVal.value = val
        Return retVal
    End Operator
    '
    Public Shared Widening Operator CType(ByVal val As FundDetail) As String
        Return val.value
    End Operator
End Class

Public Class CodeDetail
    Inherits FundDetail

    Public Overrides Function isValid(value As String) As Boolean
        Dim intPattern As Regex = New Regex("[a-zA-Z]{3}[0-9]{12}")
        Return intPattern.IsMatch(value)
    End Function

    Public Shared Shadows Widening Operator CType(ByVal val As String) As CodeDetail
        Dim retVal As New CodeDetail
        retVal.value = val
        Return retVal
    End Operator

    Public Shared Shadows Widening Operator CType(ByVal val As CodeDetail) As String
        Return val.value
    End Operator

End Class

And Usage:

    Dim fe As New FundElement
    fe.Name = "Google Inc"
    fe.Code = "USD451234151234"

    Dim val As New String(fe.Name)


    Console.WriteLine("{0}:{1}", CStr(fe.Name), CStr(fe.Code))

You'll notice from the usage example that whilst it can be used for String targets directly, it also doesn't default cast the object to a string when passed to something that expects an unknown / object such as the WriteLine call. So there are some definite downsides to this approach, it's whether or not the benefits are sufficient for your case.

Bug?

As I've said, I rarely write VB. However, this feels like a bug to me:

Shared Function isValid(detail As String) As Boolean
Shared Shadows Function isValid(value As String) As Boolean

You're using shared functions to determine if the strings are valid. So, if you do this:

CodeDetail.isValid("USD45123415~~~~~~~~~~1234")

The shadowing works, and the correct implementation is invoked. However, because you're invoking the implementation from the base class:

If isValid(valueToAssign) Then

Then as the base class only knows about the isValid it's implemented, it's invoking that version, not the newer version on the child. As I've said in the comments, if I do this:

fe.Code.value = "USD45123415~~~~~~~~~~1234"

No error is thrown, however if I do this:

If CodeDetail.isValid("USD45123415~~~~~~~~~~1234") Then

Then I know the error should be thrown.

If you want it to behave the way you're expecting, you need to use Overridable functions (as in my example) so that dynamic dispatch from the base class works.

\$\endgroup\$
  • \$\begingroup\$ Since we are going to be writing boilerplate code for each fundDetail, wouldn't it be more convenient to write the boilerplate properties rather than boilerplate widening conversions? \$\endgroup\$ – Tomas Zubiri Aug 4 '16 at 13:10
  • \$\begingroup\$ @JohnnyBravo Depends on the number of properties Vs the number of fundDetail types, as well as if you're using the type types for other API classes or not. Having the boiler plate around the properties is probably the less error prone approach. Personally, from the examples shown I'd probably have used strings for the values in FundElement and had the Set invoke appropriate validation, but that may be because I can't see the rest of your FundDetails classes. \$\endgroup\$ – forsvarir Aug 4 '16 at 13:33
  • \$\begingroup\$ The main reason why I didn't do that is because I needed the validation functions separate for unit testing. \$\endgroup\$ – Tomas Zubiri Aug 4 '16 at 13:38
  • \$\begingroup\$ @JohnnyBravo I'm assuming that what you're doing is testing the isValid method on each of your classes, then doing something like validating that fields have been declared with the right type? I would argue that this isn't the right approach (and possibly isn't working for you). At the moment, with your code, I don't think the custom validator is actually being invoked (because you're using shared, not overidable). I can do fe.Code.value = "USD45123415~~~1234" without an exception being thrown with your existing code, which seems wrong... \$\endgroup\$ – forsvarir Aug 4 '16 at 13:57
  • \$\begingroup\$ fe.code would be a codeDetail , which has an isValid function which overrides isValid with the shadows keyword. \$\endgroup\$ – Tomas Zubiri Aug 4 '16 at 14:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.