This isn't a bad approach - simple, effective, and gets the job done. It could be ever-so-slightly better, though.
First, string concatenation (str1 & str2
, str1 + str2
) in .NET has been known to be extremely slow at times, due to extra allocations. (Basically, str1 + str2 + str3 + str4
creates a temp string str12
, then creates another temp string str123
and then creates a final string str1234
. Doing this in a loop means that it creates a temp string returnStringStr1234
then sets returnString
to that, and then repeats all four of these new allocations for each iteration, at a minimum.) There's something in the framework called StringBuilder
, and I'd like to recommend you look into it.
I should also mention that we also use StringBuilder
from a semantics point of view: instead of wondering what returnString
does (is it a bunch of sub-strings, is it just one string being set by a certain loop iteration, etc.) we have something that indicates that our return value should be building a string. Everything starts to make sense.
Essentially, the StringBuilder
has a sort of internal buffer that, instead of creating a new copy of the strings, it just adds the strings together. This also exposes a few useful properties, one of which being Length
, which can be increased or decreased, removing the need for Left(...)
.
Dim result As New StringBuilder()
For Each kvp in oParameters
result.Append(kvp.Key).Append("="c).Append(kvp.Value).Append("&"c)
Next
result.Length -= 1
Return result.ToString()
This is only one line-of-code longer than your version, but it should perform more quickly and is also clearer about what the last step is (remove the last character). Of course, an alternative implementation that avoids manipulating the StringBuilder
like that might look like:
Dim result As New StringBuilder()
For Each kvp In oParameters
If result.Length > 0 Then result.Append("&"c)
result.Append(kvp.Key).Append("="c).Append(kvp.Value)
Next
Return result.ToString()
In this situation we test if the Length
is > 0
to see if it already has an element. If so, we append the ampersand ("&"c
) and continue on. Now we don't need to worry about the last character being an ampersand at all.
I've used a couple constructs that may be unfamiliar to you:
- The
If expression Then statement
is a single-line If
statement. It must be on one line (unless in a line-continuation context) and will do the same thing as If expression Then <NEWLINE> statement <NEWLINE> End If
, but is much more succinct. I use them for guard-clauses like this a lot.
- The
"_"c
construct creates a character literal from the specified string (which should be one character only). The c
is the magic bit, you can remove it from everything if you like.
I would also recommend adding the explicit type for your kvp
:
For Each kvp As KeyValuePair(Of String, String) In oParameters
...
Next
This makes it much more clear to the compiler as to what type you expect, and tells future programmers that you do know what you're doing.
Overall, good work. Hopefully this is somewhat educational for you and not just a bunch of random nonsense. :)