Property is declared WriteOnly
ID |
vbnet.maintainability.properties_should_not_be_write_only |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Api Design |
Language |
VB.NET |
Description
Reports a property declared with the WriteOnly modifier, which gives it a Set accessor and
no Get. Both ways of writing the declaration are covered, with and without the empty
parentheses after the property name - WriteOnly Property Password As String and
WriteOnly Property Password() As String. Properties that expose both accessors, properties
declared ReadOnly and fields are not reported, and neither is a Sub that takes the value as
a parameter, which is the shape this rule asks for instead.
Rationale
A property is the syntax for a value the object has; writing to one and being unable to read it
back contradicts that at the call site. credentials.Password = value looks like state being
stored, so the next author reasonably writes If credentials.Password Is Nothing and only finds
out at compile time that the value went somewhere unreadable. The consequences are practical,
not stylistic: object initialisers, data binding, serializers and object mappers all pair a
setter with a getter, so a WriteOnly property round-trips as a hole - the value is applied on
the way in and silently dropped on the way out, which turns into a configuration that saves and
reloads as empty. Where the reason for having no getter is that the value must not be readable -
a password, a token, a connection string - the property form advertises the opposite of that
intent; a method named for what it does states it. And where the setter validates or transforms
the value, its name cannot say so, while a Sub can.
The following code illustrates the pattern detected by this rule:
Private _destinationId As Integer
' FLAGGED: Property is declared WriteOnly
Public WriteOnly Property Password As String
Set(value As String)
_password = Protect(value)
End Set
End Property
' FLAGGED: Property is declared WriteOnly
Protected Overridable WriteOnly Property ConnectionString As String
Set(value As String)
_connectionString = value
End Set
End Property
' The VB6-era declaration form, with empty parentheses after the name. This is what
' long-lived VB.NET code is actually written in - all 85 WriteOnly properties in the
' reference corpus use it. (shape from cms App_Code/PDF.vb:636)
' FLAGGED: Property is declared WriteOnly
Public WriteOnly Property EncodeASCII85() As Boolean
Set(ByVal vData As Boolean)
_encodeAscii85 = vData
End Set
End Property
' ...and the same form with a modifier in front. (shape from kartris KartrisClasses.vb:293)Remediation
Replace the property with a Sub that takes the value as a parameter, named for the operation
it performs - SetPassword(value), Configure(connectionString). If the value genuinely is
readable state, add the Get accessor and drop WriteOnly; when only the fact that a value
was supplied should be observable, expose a separate ReadOnly property such as
HasPassword rather than the value itself.
' Before: assignable but not readable, and dropped by serializers and data binding
Public WriteOnly Property Password As String
Set(value As String)
_password = Protect(value)
End Set
End Property
' After
Public Sub SetPassword(value As String)
_password = Protect(value)
End Sub
Public ReadOnly Property HasPassword As Boolean
Get
Return Not String.IsNullOrEmpty(_password)
End Get
End Property