HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

De-serializing and serializing objects to be sent over the network

Submitted by: @import:stackexchange-codereview··
0
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:

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.

Code Snippets

public name as string
  public cmd as Commands
  public  message  as String
Dim oData as data

Context

StackExchange Code Review Q#88728, answer score: 4

Revisions (0)

No revisions yet.