Problem
I am serializing and de-serializing a class object within the class which is then sent over the network like this:
class data
public name as string
public cmd as Commands
public message as String
Public Sub New(ByVal byteData As Byte())
Try
' convert byte array to memory stream
Using _ms As New MemoryStream(byteData)
' create new BinaryFormatter
Dim _bf As New BinaryFormatter()
' set memory stream position to starting point
_ms.Position = 0
' Deserializes a stream into an object graph and return as a object.
dim oData as data =_bf.Deserialize(_ms)
name = oData.name
message = oData.Message
cmd = oData.cmd
End Using
Catch ex As Exception
Throw
End Try
End Sub
'Converts the Data structure into an array of bytes
Public Function ToByte() As Byte()
Dim bf As New BinaryFormatter()
Try
Using ms As New MemoryStream()
bf.Serialize(ms, Me)
Return ms.ToArray()
End Using
Catch ex As SerializationException
Return Nothing
Finally
bf = Nothing
End Try
End Function
end class
Is this proper and efficient?
Solution
IMHO you are doing to much inside of the constructor (Sub New()). The constructor should be used to construct the object. It should be avoided that exceptions are thrown from inside of a constructor.
A better approach would be to add a Shared method which takes care of the deserializing and make the constructor private. This shared method would then return the fully constructed data object.
The exception handling in the constructor is senseless in its current state. You don’t gain any advantage of using a Try..Catch because you are throwing the exception anyway. You can simply omit it.
Instead of calling ms.ToArray() in the ToByte() method you sould call ms.GetBuffer() which returns the content of the stream without creating a new byte array.
There is no need to set the Position property of the newly created MemoryStream. It is automatically set to the beginning. So skip _ms.Position = 0.
While we are at _ms, one shouldn’t use underscore prefix unless it is a class level variable where it is accteptable.
public name as string
public cmd as Commands
public message as String
These fields shouldn’t be public avaible. For encapsulation you should use properties.
Comments should be used to explain why something is done in the way it is done. Let the code speak for itself about what is done by using meaningful and descriptive names for classes, methods and variables.
Comments like ' create new BinaryFormatter only add noise to the code which should be removed for readability.
In the ToByte() method the setting of bf = Nothing is not needed and therefor you should declare the BinaryFormatter as near to its usage as possible (inside the using).
The Catch ex As SerializationException would need a comment why you handle the exception like you do.
This 'Converts the Data structure into an array of bytes should be put inside a XML documentation block.
Dim oData as data
Please don’t use hungarian notation like you are using it. By having intellisense you can easily hoover over the variable and see it is a class. There is no need to prefix it with o for object.
Naming a class data does not add any value. One should name a class in a way that it is obvious what purpose the class serves.
You also should be consistent at your naming style. Sometimese you are using an underscore prefix (like already stated) and sometimes you don’t. If you have choosen a style you should stick to it.
I get the distinct feeling that this should be a set of extension methods instead of a constructor or regular method. It has that code smell about it.
MSDN Reference of Extension Methods
Also, you should use more proper naming conventions. (Name instead of name, etc.) And you don’t need those Try/Catch/Finally blocks. Lastly, you seem to be missing the <Serializable> attribute.
Take your initial code
class data
public name as string
public cmd as Commands
public message as String
Public Sub New(ByVal byteData As Byte())
Try
' convert byte array to memory stream
Using _ms As New MemoryStream(byteData)
' create new BinaryFormatter
Dim _bf As New BinaryFormatter()
' set memory stream position to starting point
_ms.Position = 0
' Deserializes a stream into an object graph and return as a object.
dim oData as data =_bf.Deserialize(_ms)
name = oData.name
message = oData.Message
cmd = oData.cmd
End Using
Catch ex As Exception
Throw
End Try
End Sub
'Converts the Data structure into an array of bytes
Public Function ToByte() As Byte()
Dim bf As New BinaryFormatter()
Try
Using ms As New MemoryStream()
bf.Serialize(ms, Me)
Return ms.ToArray()
End Using
Catch ex As SerializationException
Return Nothing
Finally
bf = Nothing
End Try
End Function
end class
And let’s rewrite it to be more effective.
<Serializable>
Class Data
Public Name As String
Public Cmd As Commands
Public Message As String
End Class
<Extension>
Public Sub FromByteArray(ByVal data As Data, ByVal byteArray As Byte())
Using _ms As New MemoryStream(byteArray)
' create new BinaryFormatter
Dim _bf As New BinaryFormatter()
' set memory stream position to starting point
_ms.Position = 0
' Deserializes a stream into an object graph and return as a object.
Dim oData As Data = _bf.Deserialize(_ms)
data.Name = oData.Name
data.Message = oData.Message
data.Cmd = oData.Cmd
End Using
End Sub
<Extension>
Public Function ToByteArray(ByVal data As Data)
Dim bf As New BinaryFormatter()
Using ms As New MemoryStream()
bf.Serialize(ms, data)
Return ms.ToArray()
End Using
End Function
Do note: Extension methods can only be declared at the module level in VB.NET.
This technique very gracefully separates the responsibility of the class and it’s serialization. The class should be responsible for handling it’s work, whereas the Serialization/Deserialization can be done in an extension method, which makes it appear to be part of the class, but it doesn’t need to be.
This, and the modifications by Heslacher, should get you much more desirable code. (I did not include any modifications he proposed, except the obvious code cleaning.)
Sample usage:
Sub Main()
Dim data As New Data()
data.Name = "Test"
Dim ByteArray As Byte() = data.ToByteArray()
data.FromByteArray(ByteArray)
Console.WriteLine(data.Name)
Console.ReadLine()
End Sub
That will print Test, not only proving that this method works, but also proving, I think, your original design was flawed. How do you create an empty Data object if the only constructor for it requires a Byte-Array?