patterncsharpMinor
De-serializing and serializing objects to be sent over the network
Viewed 0 times
theobjectsserializingnetworksentandover
Problem
I am serializing and de-serializing a class object within the class which is then sent over the network like this:
Is this proper and efficient?
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 classIs this proper and efficient?
Solution
IMHO you are doing to much inside of the constructor (
A better approach would be to add a
The exception handling in the constructor is senseless in its current state. You don't gain any advantage of using a
Instead of calling
There is no need to set the
While we are at
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
In the
The
This
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
Naming a class
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.
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 StringThese 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 dataPlease 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.
Code Snippets
public name as string
public cmd as Commands
public message as StringDim oData as dataContext
StackExchange Code Review Q#88728, answer score: 4
Revisions (0)
No revisions yet.