Redundant string object created from a value that is already the string wanted
ID |
vbnet.performance.avoid_unnecessary_string_creation |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
String Handling |
Language |
VB.NET |
Description
Reports expressions that allocate a new string equal to one that already exists:
New String(s.ToCharArray()) and New StringBuilder(s).ToString(), which copy a string out
and straight back in; String.Copy(s), which exists only to force a distinct instance;
ToString() applied to a string literal or to the result of another ToString(); and
String.Concat of two literals, which the compiler would have folded had they been written as
one. Genuine uses of the same APIs are accepted - New String(" "c, width) builds a repeated
character, and a StringBuilder that appends before calling ToString() is doing real work.
Rationale
Strings in .NET are immutable, so a copy of a string is never needed for safety: nothing can
change the original through a shared reference. Each of these forms allocates a fresh object
on the heap, copies the characters into it, and produces a value indistinguishable from the
input, so the whole expression costs an allocation and a copy for no observable effect. On a
code path that runs per request or per row that allocation is pure garbage-collector pressure,
and it is invisible in profiles because no single call is slow. There is a correctness angle
too: because the copy is a distinct object, reference comparison stops agreeing with value
comparison, so ReferenceEquals(s, String.Copy(s)) is False and any code that relies on
interning - or on Is rather than = - starts behaving differently for no reason the reader
can see. String.Copy is marked obsolete in modern .NET for exactly this reason. The literal
cases are simpler still: the compiler already knows the answer, so "INV-".ToString() and
String.Concat("INV", "-") ask for work at run time that could have been written as a single
constant.
The following code illustrates the pattern detected by this rule:
Public Function Duplicate(ByVal reference As String) As String
' FLAGGED: Redundant string object created from a value that is already the string wanted
Return New String(reference.ToCharArray())
End FunctionRemediation
Use the string you already have. Delete the New String(…), String.Copy(…) or
New StringBuilder(…).ToString() wrapper and pass the original value; drop ToString() from
anything that is already a string; and write two adjacent literals as one literal, or as a
Const, so the value is fixed at compile time. Keep a StringBuilder only where several
pieces are appended - for a single concatenation the & operator or an interpolated string is
both shorter and cheaper.
' Before: an allocation and a copy that produce an identical value
Return New String(reference.ToCharArray())
Return String.Copy(reference)
Return "INV-".ToString()
Return String.Concat("INV", "-")
' After
Return reference
Return "INV-"