10
\$\begingroup\$

Here's a brief explanation of my method:

  1. Provide a string and parse it to HTML code respecting a specific format.
  2. The accepted format is:

    For dropdown menu : Test DropDown~Select:Options1; Options2;Options3--- --- For Text area: Test Text area~TextArea:text to display**

I am inheriting this VB code from a legacy system we are using right now and trying to replace with a C# system. Though the naming convention is off, it is not how I declare my variables/methods (look at my C# unit testing code to have an idea).

How I can maintain the tests? We change the CSS classes a lot, which means I have to change the produced HTML, which means changing my tests. Is that ok?

Public Function BuildHtmlControlResponsive(sourceText As String, indexNum As Integer) As String
      Dim out As StringBuilder = New StringBuilder
      'To store the text after the "~" 
      Dim textSelection As String
      Dim tildeLocation As Integer
      'To store the text of the control to parse (EX:Text Area, Input , Options menu ...)
      Dim controlOption As String = String.Empty

      'To store the name the text of the label
      Dim textLabel As String = String.Empty
      'To store the list of options (If applicable)
      Dim contentText As String = String.Empty
      tildeLocation = sourceText.IndexOf("~"c)
      If (tildeLocation > -1) Then
        'This gets the text that comes after the ~. Example Company~TextArea would return TextArea. 
        textSelection = sourceText.Substring((tildeLocation + 1), (sourceText.Length - (tildeLocation + 1)))
        textLabel = sourceText.Substring(0, sourceText.IndexOf("~"c))

        'See if the string has the : sign
        If (textSelection.IndexOf(":"c) > -1) Then
          'This gets the text that comes before the : Example Company:TextArea would return TextArea. 
          controlOption = textSelection.Substring(0, textSelection.IndexOf(":"c))
          'This gets the text that comes After the : Example Company:TextArea would return TextArea. 
          contentText = textSelection.Substring(textSelection.IndexOf(":"c) + 1)
        End If

      End If

      out.Append("<div class=""form-group"">")
      out.Append("<label for=""extField_" & indexNum & """ class=""col-md-4 control-label"" id=""extLabelField_" & indexNum & """ >" & textLabel & ":</label>" & vbCrLf)
      out.Append("<div class=""col-md-8"">")

      If (textSelection <> String.Empty) Then
        Select Case controlOption.ToUpper()

          Case "TEXTAREA", "TEXT AREA"
            out.Append("<textarea id=""extField_" & indexNum & """ class=""form-control"" type=""text"" rows=""3""  placeholder=""" & textLabel & """> </textarea> ")

          Case "SELECT"
            If (sourceText.IndexOf(";"c) <> -1) Then
              ' list box.  Syntax: name ; option 1 ; option 2 ; ...
              Dim OptionList As String()
              Dim i As Integer
              OptionList = contentText.Split(";"c)

              out.Append("<select class=""form-control""  id=""extField_" & indexNum & """>")
              For i = 0 To UBound(OptionList)
                out.Append("<option>" & OptionList(i) & "</option>")
              Next
              out.Append("</select>")
            Else
              ' text box. Syntax: name
              out.Append("<input type=""text""  class=""form-control"" id=""extField_" & indexNum & """  Columns=""18"" class=""form"" />" & vbCrLf)
            End If
          Case Else
            out.Append("<input type=""text"" class=""form-control"" id=""extField_" & indexNum & """ placeholder=""" & textLabel & """>")

        End Select
      Else
        'Defaults to outputting a text field if nothing is specified. 
        out.Append("<input type=""text"" class=""form-control"" id=""extField_" & indexNum & """ placeholder=""" & textLabel & """>")
      End If
      out.Append("</div>")
      out.Append("</div>")

      Return out.ToString()
    End Function

Unit tests

[TestMethod]
public void ResponsiveEPriceCustomFieldsOptionsTestMethod()
{

   //Initialize
    string inputStringOptions = "Test~Select: option1;option2;Option3;option4";


    //arrange
    UCSendePrice UCsendPriceInstance = new userControl_sendeprice_r1();

    string ExpectedHTMLResultOptions = @"<div class=""form-group""><label for=""extField_1"" class=""col-md-4 control-label"" id=""extLabelField_1"" >Test:</label>
<div class=""col-md-8""><select class=""form-control""  id=""extField_1""><option> option1</option><option>option2</option><option>Option3</option><option>option4</option></select></div></div>";


    //Act
    string actualHTMLCodeOptions = UCsendPriceInstance.BuildHtmlControlResponsive(inputStringOptions, 1);

    //Assert
    Assert.AreEqual(ExpectedHTMLResultOptions, actualHTMLCodeOptions);
}


[TestMethod]
public void ResponsiveEPriceCustomFieldsTextAreaTestMethod()
{

    //Initialize
    UCSendePrice uCsendPriceInstance = new userControl_sendeprice_r1();
    string inputStringTextArea = "Test~TextArea: option1;option2;Option3;option4";
    //arrange
    string ExpectedHTMLResultTextArea = @"<div class=""form-group""><label for=""extField_1"" class=""col-md-4 control-label"" id=""extLabelField_1"" >Test:</label>
<div class=""col-md-8""><textarea id=""extField_1"" class=""form-control"" type=""text"" rows=""3""  placeholder=""Test""> </textarea> </div></div>";


    //Act
    string actualHTMLCodeTextArea = uCsendPriceInstance.BuildHtmlControlResponsive(inputStringTextArea, 1);

    //Assert
    Assert.AreEqual(ExpectedHTMLResultTextArea, actualHTMLCodeTextArea);
}


[TestMethod]
public void ResponsiveEPriceCustomFieldsInputTextTestMethod()
{

    //Initialize
    UCSendePrice uCsendPriceInstance = new userControl_sendeprice_r1();
    string inputStringInputText = "Test Input~ option1;option2;Option3;option4";


    //arrange
    string ExpectedHTMLResultInputText = @"<div class=""form-group""><label for=""extField_1"" class=""col-md-4 control-label"" id=""extLabelField_1"" >Test Input:</label>
<div class=""col-md-8""><input type=""text"" class=""form-control"" id=""extField_1"" placeholder=""Test Input""></div></div>";


    //Act
    string actualHTMLCodeInputText = uCsendPriceInstance.BuildHtmlControlResponsive(inputStringInputText, 1);

    //Assert
    Assert.AreEqual(ExpectedHTMLResultInputText, actualHTMLCodeInputText);
}
\$\endgroup\$
8
+25
\$\begingroup\$

Before we start, take a look at this comment of yours. It's a prime example of why you shouldn't always trust the comments.

'This gets the text that comes before the : 
'Example Company:TextArea would return TextArea. 
controlOption = textSelection.Substring(0, textSelection.IndexOf(":"c))

The text before : is in fact Company ;)


Now, the name of the method BuildHtmlControlResponsive, IMHO "responsive" is referring the word "build. The builder is run in responsive mode and creates a HtmlControl.

Public Function BuildHtmlControlResponsive(sourceText As String, indexNum As Integer) As String

In reality you're building a ResponsiveHtmlControl. So I suggest you rename the method. I also suggest you rename the parameters. (Please don't abbreviate field/method names)

Public Function BuildResponsiveHtmlControl(command As String, index As Integer) As String

Syntax

You need to define the syntax. This is how I read the rules:

{ labelText~ [ inputType: [ selectOption [;....n] ] ] }

  • { } (braces) Required syntax items. Do not type the braces.
  • [ ] (brackets) Optional syntax items. Do not type the brackets.
  • [;...n] Indicates the preceding item can be repeated n number of times. The occurrences are separated by semicolons.

The name of some of your variables makes no sense. I took the liberty to change all of them. You should also store the character position returned from IndexOf to avoid duplicate calls. And there's a bug in your code. What if there's only one option? Aka. no semicolon?

Test~TextArea: option1

If we apply all the changes/fixes, the code will look like this:

Dim labelText As String = String.Empty
Dim inputType As String = String.Empty
Dim selectOptions As String() = Nothing
Dim position As Integer = command.IndexOf("~")

If (position > -1) Then

    Dim cursor As Integer = (position + 1)

    labelText = command.Substring(0, position)
    position = command.IndexOf(":", cursor)

    If (position > -1) Then

        inputType = command.Substring(cursor, (position - cursor)).ToLower()
        cursor = (position + 1)

        If ((inputType = "select") AndAlso (cursor < command.Length)) Then
            selectOptions = command.Substring(cursor).Split(";"c)
        End If

    End If

End If

HTML

HTML code is usually written in lowercase, so I suggest you change it accordingly.

controlOption.ToUpper()

The input element doesn't have an attribute named Columns. And you cannot have two identical attributes (you have two class attributes).

out.Append("<input type=""text""  class=""form-control"" id=""extField_" & indexNum & """  Columns=""18"" class=""form"" />" & vbCrLf)

Why do you default to an input element if there are only one or less options? It makes no sense. I suggest you return an empty select element.

Case "SELECT"
    If (sourceText.IndexOf(";"c) <> -1) Then
        ...
    Else
        ' text box. Syntax: name
        out.Append("<input type=""text""  class=""form-control"" id=""extField_" & indexNum & """  Columns=""18"" class=""form"" />" & vbCrLf)
    End If

It's pretty clear that you have a maintainability issue, but you also have a readability issue. If speed isn't that big of concern, then I have a great suggestion. VB.Net has a great support for writing XML in the code editor. But please note that XML is not HTML.

Again, if we apply all the changes/fixes, the code will look like this:

Dim elementId As String = String.Format("extField_{0}", index)
Dim labelId As String = String.Format("extLabelField_{0}", index)
Dim element As XElement = Nothing

Select Case inputType
    Case "textarea"
        element = <textarea class="form-control" rows="3" id=<%= elementId %> placeholder=<%= labelText %>></textarea>
    Case "select"
        element = <select id=<%= elementId %>></select>
        Array.ForEach(If(selectOptions, {}), Sub(item) element.Add(<option><%= item %></option>))
    Case Else
        element = <input type="text" id=<%= elementId %> placeholder=<%= labelText %>/>
End Select

Dim root =
    <div class="form-group">
        <label class="control-label col-md-4" for=<%= elementId %> id=<%= labelId %>>Test:</label>
        <div class="col-md-8"></div>
    </div>

root.Element("div").Add(element)

Return root.ToString()

The code looks even better when viewed in Visual Studio.

Code

Unit test

I'm not sure why you use C# for unit testing, but if you can switch to VB.Net, here's how easy it is to write (and read) the expected result (assuming the index = 1234):

Dim command As String = "Test~select:option1;option2;option3;option4"

Dim expectedResult As String =
    <div class="form-group">
        <label class="control-label col-md-4" for="extField_1234" id="extLabelField_1234">Test:</label>
        <div class="col-md-8">
            <select id="extField_1234">
                <option>option1</option>
                <option>option2</option>
                <option>option3</option>
                <option>option4</option>
            </select>
        </div>
    </div>.ToString()

Print screen of Visual Studio code editor:

Code

Source code

Here's the complete method body:

Public Function BuildResponsiveHtmlControl(command As String, index As Integer) As String

    Dim labelText As String = String.Empty
    Dim inputType As String = String.Empty
    Dim selectOptions As String() = {}
    Dim position As Integer = command.IndexOf("~")

    If (position > -1) Then

        Dim cursor As Integer = (position + 1)

        labelText = command.Substring(0, position)
        position = command.IndexOf(":", cursor)

        If (position > -1) Then

            inputType = command.Substring(cursor, (position - cursor)).ToLower()
            cursor = (position + 1)

            If ((inputType = "select") AndAlso (cursor < command.Length)) Then
                selectOptions = command.Substring(cursor).Split(";"c)
            End If

        End If

    End If

    Dim elementId As String = String.Format("extField_{0}", index)
    Dim labelId As String = String.Format("extLabelField_{0}", index)
    Dim element As XElement = Nothing

    Select Case inputType
        Case "textarea"
            element = <textarea class="form-control" rows="3" id=<%= elementId %> placeholder=<%= labelText %>></textarea>
        Case "select"
            element = <select id=<%= elementId %>></select>
            Array.ForEach(If(selectOptions, {}), Sub(item) element.Add(<option><%= item %></option>))
        Case Else
            element = <input type="text" id=<%= elementId %> placeholder=<%= labelText %>/>
    End Select

    Dim root =
        <div class="form-group">
            <label class="control-label col-md-4" for=<%= elementId %> id=<%= labelId %>>Test:</label>
            <div class="col-md-8"></div>
        </div>

    root.Element("div").Add(element)

    Return root.ToString()

End Function
\$\endgroup\$
  • \$\begingroup\$ Thank you for your comments and recommendations. I appreciate every word . I need to mention couple of things : I am inheriting this VB code from a legacy system we are using right now and trying to replace with a C# system. though the naming convention is off , IT IS not how I declare my variables/methods (look at my C# unit testing code to have an idea. Again my biggest concern and what i really want out of this question is how to maintain the tests? we change the css classes a lot which means i have to change the produced HTML which means changing my tests. is that ok \$\endgroup\$ – IndieTech Solutions May 15 '15 at 18:41
  • 1
    \$\begingroup\$ Ah, I see. You should add this to your question. And you should probably replace one of the tags with c#. \$\endgroup\$ – Bjørn-Roger Kringsjå May 15 '15 at 18:51

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.