HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Rendering an HTML form based on a specification in a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
specificationbasedformrenderingstringhtml

Problem

Here's a brief explanation of my method:

-
Provide a string and parse it to HTML code respecting a specific format.

-
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 retur

Solution

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

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("" & 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("" & 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 = placeholder=>
Case "select"
element = >
Array.ForEach(If(selectOptions, {}), Sub(item) element.Add())
Case Else
element = placeholder=/>
End Select

Dim root =

id=>Test:



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

Return root.ToString()


The code looks even better when viewed in Visual Studio.

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 =

Test:


option1
option2
option3
option4


.ToString()


Print screen of Visual Studio code editor:

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 placeholder=>
Case "select"
element = >
Array.ForEach(If(selectOptions, {}), Sub(item) element.Add())

Context

StackExchange Code Review Q#90562, answer score: 8

Revisions (0)

No revisions yet.