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

Objects sending TCP Client

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
clientsendingtcpobjects

Problem

I'd like to have this code looked over because I feel my approach might be very novice and there has to be a more elegant or at least less "grunt-work" way of doing it.

I have to write a small TCP client in my application which can send an object to a server for it to be stored in a Database. Before I send the object I remember that I was once told sending custom objects is generally bad practice unless you have a custom Stream Class (Which I don't have). So I am dissecting my objects before I send them into their respective Strings/ints/whatever else objects. I just feel my approach might not be very efficient.

The end result will be that the server can put together a Query to be executed in the database for storage.

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Smart_Journal.Familie;
using System.Net.Sockets;
using System.IO;

namespace Smart_Journal
{
///
/// DBM is our Database Manager. It will handle sending and receiving data from the server.
/// You should NOT call these functions directly. Instead, call the functions in the Utility Class.
///
public static class DBM
{
private static TcpClient _client;
private static StreamReader _sReader;
private static StreamWriter _sWriter;
private static bool _isConnected;
private static int portNum;
private static String ipAddress;

private static void InitConnection()
{
_client = new TcpClient();
_client.Connect(ipAddress, portNum);

_sReader = new StreamReader(_client.GetStream(), Encoding.ASCII);
_sWriter = new StreamWriter(_client.GetStream(), Encoding.ASCII);

_isConnected = true;
}

private static void EndConnection()
{
_sWriter.WriteLine("END");
_sWriter.Flush();
_sReader.Close();
_sWriter.Close();
_cli

Solution

I find whenever I feel the need to use a #region, there's something fishy going on with my code; I see the same in your SendPlejeFamilieObjekt method: all those WriteLine/Flush calls look very suspicious. Why do you flush at every single line written?

Have you considered overriding PlejeFamilie.ToString() to return the equivalent string? You could do _sWriter.WriteLine(pfamilie.ToString(); and have no code to modify in that method the day PlejeFamilie needs a new member.

[scrolls down]

The same applies to BiologiskFamilie; whenever something changes in these classes, you have multiple places that must change. I would refactor this code so as to minimize the amount of code that needs to be written (therefore possible omissions and bugs) when a change becomes required.

Why is everything static? This class has disposable private fields, I think it should be an instance, and should implement IDisposable so it could be used in a using block and then the client code doesn't need to "remember" to call EndConnection.

Context

StackExchange Code Review Q#36207, answer score: 3

Revisions (0)

No revisions yet.