• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
Xanather

Quality of my code

7 posts in this topic

I haven't done much network programming lately so I decided to make a small program. I quickly made this small program within 2 days, it uses .net's TcpClient and TcpListener classes.

 

All the program does is transfer files to/from computers. Server mode of the program accepts files, while client mode of the program sends files.

 

I would like any experienced (networking) programmers to tell me what I should improve on.

 

Looking at the code now I would:

 

1. try and somehow make reading the file and uploading the file happen on different threads (which will probably make the transfer faster). Same with downloading/saving the file.

 

2. split the code up a bit mode, its mostly crammed into one method - how would I do this though?

 

Here are the 4 classes used in the program:

 

Program.cs...

//By xanather.
using System;

namespace Transfar
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.Title = "Transfar - Clean and Fast TCP File Transfers";
            Console.WriteLine("Loaded: Transfar v1.0");
            if (args.Length == 0)
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("This executable needs to be passed some arguments...");
                Console.ForegroundColor = color;
                ShowHelp();
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            else
            {
                if (args[0] == "-server")
                {
                    Server server;
                    server = new Server();
                    server.Start(args);
                }
                else if (args[0] == "-client")
                {
                    Client client;
                    client = new Client();
                    client.Start(args);
                }
                else
                {
                    ConsoleColor color = Console.ForegroundColor;
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine("Invalid argument(s). Here is the list of arguments...");
                    Console.ForegroundColor = color;
                    ShowHelp();
                    Console.Write("Press any key to close this program... ");
                    Console.ReadKey();
                    return;
                }
            }
        }
        static void ShowHelp()
        {
            Console.WriteLine("Available arguments (commands) for this application:");
            Console.WriteLine("Argument\t\tDescription");
            Console.WriteLine("-server [dir] [port]\n");
            Console.WriteLine("\t\t\tStarts the application in server mode so the");
            Console.WriteLine("\t\t\tapplication can receive and download files sent by");
            Console.WriteLine("\t\t\tTransfar clients. Port forwarding on the router");
            Console.WriteLine("\t\t\tmay be required so that the server can listen for");
            Console.WriteLine("\t\t\tinternet connections.\n");
            Console.WriteLine("\t\t\tThe [dir] argument specifies the folder/directory");
            Console.WriteLine("\t\t\tthe server should place received/downloaded files");
            Console.WriteLine("\t\t\tfrom clients. This argument is required.\n");
            Console.WriteLine("\t\t\tThe [port] argument specifies the port in which the");
            Console.WriteLine("\t\t\tserver should listen to for accepting Transfer client");
            Console.WriteLine("\t\t\tconnections. Default port is 5655. This argument is");
            Console.WriteLine("\t\t\toptional.\n");
            Console.WriteLine("-client [address] [file] [port]\n");
            Console.WriteLine("\t\t\tStarts the application in client mode so the");
            Console.WriteLine("\t\t\tapplication can send and upload files to a Transfar");
            Console.WriteLine("\t\t\tserver.\n");
            Console.WriteLine("\t\t\tThe [address] argument specifies the server's IP");
            Console.WriteLine("\t\t\tin which to connect to. This argument is required.\n");
            Console.WriteLine("\t\t\tThe [file] argument specifies the file on disk");
            Console.WriteLine("\t\t\tthe client should send/upload to the server.");
            Console.WriteLine("\t\t\tThis argument is required.\n");
            Console.WriteLine("\t\t\tThe [port] argument specifies which port the client");
            Console.WriteLine("\t\t\tshould use to connect to Transfar servers of the same");
            Console.WriteLine("\t\t\tport. Default port is 5655. This argument is optional.\n");
        }
    }
}

ArrayHelper.cs...

//By xanather
using System;

namespace Transfar
{
    static class ArrayHelper
    {
        public static bool Equals(byte[] a, byte[] b)
        {
            if (a.Length != b.Length)
            {
                return false;
            }
            for (int i = 0; i < a.Length; i++)
            {
                if (a[i] != b[i])
                {
                    return false;
                }
            }
            return true;
        }
    }
}

Client.cs...

//By xanather
using System;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Threading;
using System.Text;

namespace Transfar
{
    class Client
    {
        public static readonly byte[] clientVerify = new byte[] { 6, 2, 5, 1, 255 };
        public string srcFile;
        public int port;
        public string address;
        public void Start(string[] args)
        {
            //get address argument
            if (args.Length > 1)
            {
                address = args[1];
            }
            else
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("No IP address has been entered.");
                Console.WriteLine("It is a required argument.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            //get file argument
            if (args.Length > 2)
            {
                if (File.Exists(args[2]))
                {
                    srcFile = args[2];
                }
                else
                {
                    ConsoleColor color = Console.ForegroundColor;
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine("The file specified does not exist.");
                    Console.WriteLine("It is a required argument.");
                    Console.ForegroundColor = color;
                    Console.Write("Press any key to close this program... ");
                    Console.ReadKey();
                    return;
                }
            }
            else
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("No file location has been entered.");
                Console.WriteLine("It is a required argument.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            if (args.Length > 3)
            {
                try
                {
                    port = int.Parse(args[3]);
                    if (port < 0 || port > 65565)
                    {
                        throw new FormatException();
                    }
                }
                catch (FormatException)
                {
                    ConsoleColor color = Console.ForegroundColor;
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine("Invalid port argument...");
                    Console.ForegroundColor = color;
                    Console.Write("Press any key to close this program... ");
                    Console.ReadKey();
                    return;
                }
            }
            else
            {
                port = Server.defaultPort;
            }
            //check the size of the file
            try
            {
                long fileSize = checked(new FileInfo(srcFile).Length);
            }
            catch (OverflowException)
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("The file is too big! The maximum size supported is " + long.MaxValue + " bytes.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            ProcessConnection();
        }
        public void ProcessConnection()
        {
            TcpClient client;
            NetworkStream stream;
            try
            {
                Console.WriteLine("Attempting to connect...");
                client = new TcpClient(address, port);
                stream = client.GetStream();
            }
            catch (SocketException e)
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("Cannot connect to server.");
                Console.WriteLine(e.Message);
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            Console.WriteLine("Connected to server.");
            int readSize;
            int readAmount;
            byte[] readBuffer;
            int readAmountTotal;
            byte[] writeBuffer;
            byte[] writeBuffer1;
            //send client verification
            stream.Write(clientVerify, 0, clientVerify.Length);
            //read server verification
            readAmount = Server.serverVerify.Length;
            readBuffer = new byte[readAmount];
            readAmountTotal = readAmount;
            while (readAmount != 0)
            {
                readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                readAmount -= readSize;
            }
            if (!ArrayHelper.Equals(readBuffer, Server.serverVerify))
            {
                stream.Close();
                client.Close();
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("Disconnected, invalid server.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            //write file name
            string fileName = Path.GetFileName(srcFile);
            writeBuffer1 = Encoding.UTF8.GetBytes(fileName);
            writeBuffer = new byte[writeBuffer1.Length + 1];
            writeBuffer[0] = (byte)writeBuffer1.Length;
            Buffer.BlockCopy(writeBuffer1, 0, writeBuffer, 1, writeBuffer1.Length);
            stream.Write(writeBuffer, 0, writeBuffer.Length);
            //read if file already exists in server directory
            stream.Read(readBuffer, 0, 1);
            if (readBuffer[0] == 0)
            {
                stream.Close();
                client.Close();
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("Disconnected, the file {0} already exists in the server directory.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            //write the size (in bytes) of the file
            Console.WriteLine("Now uploading the file...");
            long fileSize = new FileInfo(srcFile).Length;
            stream.Write(BitConverter.GetBytes(fileSize), 0, 8);
            //start uploading the whole file
            long writeRemaining = fileSize;
            int percent = 0;
            int prevPercent = -1;
            using (FileStream fileStream = new FileStream(srcFile, FileMode.Open))
            {
                while (writeRemaining != 0)
                {
                    readAmount = short.MaxValue; //write 65565 chuncks.
                    if (readAmount > writeRemaining)
                    {
                        readAmount = (int)writeRemaining;
                    }
                    readBuffer = new byte[readAmount];
                    fileStream.Read(readBuffer, 0, readAmount);
                    try
                    {
                        stream.Write(readBuffer, 0, readAmount);
                    }
                    catch (IOException e)
                    {
                        stream.Close();
                        client.Close();
                        ConsoleColor color = Console.ForegroundColor;
                        Console.ForegroundColor = ConsoleColor.Cyan;
                        Console.WriteLine("Disconnected: " + e.Message);
                        Console.ForegroundColor = color;
                        Console.Write("Press any key to close this program... ");
                        Console.ReadKey();
                        return;
                    }
                    writeRemaining -= readAmount;
                    percent = (int)(((fileSize - writeRemaining) / (double)fileSize * 100));
                    if (percent != prevPercent)
                    {
                        prevPercent = percent;
                        Console.WriteLine("{0} - Upload at {1}%. {2}KB remaining ({3}KB/{4}KB).", fileName, percent, writeRemaining / 1024, (fileSize - writeRemaining) / 1024, fileSize / 1024);
                    }
                }
            }
            //write clientVerify for final handshake
            stream.Write(clientVerify, 0, clientVerify.Length);
            stream.Close();
            client.Close();
            Console.WriteLine("Upload complete!");
            Console.Write("Press any key to close this program... ");
            Console.ReadKey();
        }
    }
}

Server.cs...

//By xanather
using System;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Threading;
using System.Text;

namespace Transfar
{
    class Server
    {
        public static readonly byte[] serverVerify = new byte[]{10, 10, 35, 244, 21};
        public const int defaultPort = 5655;
        public int port;
        public string destDirectory;
        public AutoResetEvent connectionSignal = new AutoResetEvent(false);
        public TcpListener listener;
        public void Start(string[] args)
        {
            //get directory argument
            if (args.Length > 1)
            {
                if (!args[1].EndsWith(Path.DirectorySeparatorChar.ToString()))
                {
                    args[1] += Path.DirectorySeparatorChar;
                }
                if (Directory.Exists(args[1]))
                {
                    destDirectory = args[1];
                }
                else
                {
                    ConsoleColor color = Console.ForegroundColor;
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine("The directory specified does not exist.");
                    Console.WriteLine("It is a required argument.");
                    Console.ForegroundColor = color;
                    Console.Write("Press any key to close this program... ");
                    Console.ReadKey();
                    return;
                }
            }
            else
            {
                ConsoleColor color = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.WriteLine("No destination directory has been entered.");
                Console.WriteLine("It is a required argument.");
                Console.ForegroundColor = color;
                Console.Write("Press any key to close this program... ");
                Console.ReadKey();
                return;
            }
            //get port argument
            if (args.Length > 2)
            {
                try
                {
                    port = int.Parse(args[2]);
                    if (port < 0 || port > 65565)
                    {
                        throw new FormatException();
                    }
                }
                catch (FormatException)
                {
                    ConsoleColor color = Console.ForegroundColor;
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine("Invalid port argument...");
                    Console.ForegroundColor = color;
                    Console.Write("Press any key to close this program... ");
                    Console.ReadKey();
                    return;
                }
            }
            else
            {
                port = defaultPort;
            }
            StartListening();
        }
        public void StartListening()
        {
            listener = new TcpListener(IPAddress.Any, port);
            listener.Start();
            while (true)
            {
                Console.WriteLine("Listening for connections...");
                ProcessClientAsync();
                connectionSignal.WaitOne();
                connectionSignal.Reset();
            }
        }
        public async void ProcessClientAsync()
        {
            TcpClient client = await listener.AcceptTcpClientAsync();
            NetworkStream stream = client.GetStream();
            connectionSignal.Set();
            //connection variables
            string socketAddress = ((IPEndPoint)client.Client.RemoteEndPoint).Address.ToString();
            int socketPort = int.Parse(((IPEndPoint)client.Client.RemoteEndPoint).Port.ToString());
            int readSize;
            int readAmount;
            byte[] readBuffer;
            int readAmountTotal;
            //verify client connection
            readAmount = Client.clientVerify.Length;
            readBuffer = new byte[readAmount];
            readAmountTotal = readAmount;
            while (readAmount != 0)
            {
                readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                readAmount -= readSize;
            }
            if (!ArrayHelper.Equals(readBuffer, Client.clientVerify))
            {
                stream.Close();
                client.Close();
                Console.WriteLine(string.Format("[{0}:{1}] Warning: Invalid connection request by this IP.", socketAddress, socketPort));
                return;
            }
            //send server verification
            stream.Write(serverVerify, 0, serverVerify.Length);
            Console.WriteLine(string.Format("[{0}:{1}] Connected.", socketAddress, socketPort));
            //read file name
            stream.Read(readBuffer, 0, 1);
            readAmount = readBuffer[0];
            readBuffer = new byte[readAmount];
            readAmountTotal = readAmount;
            while (readAmount != 0)
            {
                readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                readAmount -= readSize;
            }
            string fileName = Encoding.UTF8.GetString(readBuffer);
            //write if file already exists in destDirectory
            if (File.Exists(destDirectory + fileName))
            {
                stream.Write(new byte[] { 0 }, 0, 1);
                stream.Close();
                client.Close();
                Console.WriteLine(string.Format("[{0}:{1}] Disconnected, tried to save a existing file ({2}).", socketAddress, socketPort, fileName));
                return;
            }
            else
            {
                stream.Write(new byte[] { 1 }, 0, 1);
            }
            //read the size (in bytes) of the file
            readAmount = 8;
            readBuffer = new byte[readAmount];
            readAmountTotal = readAmount;
            while (readAmount != 0)
            {
                readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                readAmount -= readSize;
            }
            long fileSize = BitConverter.ToInt64(readBuffer, 0);
            //start downloading the whole file
            long readRemaining = fileSize;
            int percent = 0;
            int prevPercent = -1;
            Console.WriteLine(string.Format("[{0}:{1}] Now downloading file {2}. Total size: {3}KB.", socketAddress, socketPort, fileName, fileSize / 1024));
            using (FileStream fileStream = new FileStream(destDirectory + fileName, FileMode.CreateNew))
            {
                while (readRemaining != 0)
                {
                    readAmount = short.MaxValue; //read 65565 chuncks.
                    if (readAmount > readRemaining)
                    {
                        readAmount = (int)readRemaining;
                    }
                    readBuffer = new byte[readAmount];
                    readAmountTotal = readAmount;
                    while (readAmount != 0)
                    {
                        try
                        {
                            readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                        }
                        catch (IOException e)
                        {
                            stream.Close();
                            client.Close();
                            Console.WriteLine(string.Format("[{0}:{1}] Disconnected: {2}", socketAddress, socketPort, e.Message));
                            Console.WriteLine(string.Format("[{0}:{1}] The file has not been received in full and may be corrupted.", socketAddress, socketPort, e.Message));
                            return;
                        }
                        readAmount -= readSize;
                    }
                    fileStream.Write(readBuffer, 0, readAmountTotal);
                    readRemaining -= readAmountTotal;
                    percent = (int)(((fileSize - readRemaining) / (double)fileSize) * 100);
                    if (percent != prevPercent)
                    {
                        prevPercent = percent;
                        Console.WriteLine("[{0}:{1}] {2} - Download at {3}%. {4}KB remaining ({5}KB/{6}KB).", socketAddress, socketPort, fileName, percent, readRemaining / 1024, (fileSize - readRemaining) / 1024, fileSize / 1024);
                    }
                }
            }
            //read clientVerify for final handshake
            readAmount = Client.clientVerify.Length;
            readBuffer = new byte[readAmount];
            readAmountTotal = readAmount;
            while (readAmount != 0)
            {
                readSize = stream.Read(readBuffer, readAmountTotal - readAmount, readAmount);
                readAmount -= readSize;
            }
            if (!ArrayHelper.Equals(readBuffer, Client.clientVerify))
            {
                stream.Close();
                client.Close();
                Console.WriteLine(string.Format("[{0}:{1}] Warning: Unspecified error, file may not be received properly.", socketAddress, socketPort));
                return;
            }
            else
            {
                Console.WriteLine(string.Format("[{0}:{1}] {2} - Download complete!", socketAddress, socketPort, fileName));
            }
            stream.Close();
            client.Close();
        }
    }
}

Ok, that ended up longer than I thought.

 

All replies are appriciated.

Thanks,

Xanather.

1

Share this post


Link to post
Share on other sites

Thanks for the reply.

 

Yeah I thought I should have used more methods, with the server class though having multiple methods for each stage of the connnection would mean I would need to keep passing references around (TcpClient/NetworkStream) between the methods for no reason?

 

I also find making everything public is easier to deal with, I could understand why you would do encapsulation in public libraries though?

0

Share this post


Link to post
Share on other sites

My pet peeve is not making it clear that variables are member variables, but that's more of a C++ism I guess than a C# one. The problem is someone reading your code will have a hard time telling what scope the variable is in without something like m_ or self. (as in the case of python) on the front of it. It's just a readability nitpick, but at work forgetting m_ or the less common using m_ for local scope variables is punishable by death. >:]

0

Share this post


Link to post
Share on other sites

My pet peeve is not making it clear that variables are member variables, but that's more of a C++ism I guess than a C# one. The problem is someone reading your code will have a hard time telling what scope the variable is in without something like m_ or self. (as in the case of python) on the front of it. It's just a readability nitpick, but at work forgetting m_ or the less common using m_ for local scope variables is punishable by death. >:]

 

Punishable by death?

 

The laws must be a bit more relaxed over here in Australia. LOL

0

Share this post


Link to post
Share on other sites

@ frob, thank you for your time and reply. I actually enjoyed reading your post, some of your comments actually made me laugh :D (in the good way ofcourse).

 

What's with all the colorful output?

Yea. looking back I would have never done this, your right it took up way to many lines.

 

The ArrayHelper.Equals function you included looks almost exactly like the Enumerable.SequenceEqual method. Your usage is:

bool result = ArrayHelper.Equals(a, b);

The built in SequenceEqual method would be:

bool result = a.SequenceEqual(b);

Didnt even know such thing existed, thanks (i should really look at the msdn docs more)

 

You throw a FormatException and then catch it two lines later. Why? Exceptions require significant resources and should be used for exceptional circumstances, not as a convenient way to break a loop.

I have also realized this recently, I am not doing this anymore.

 

You send a "client verification". It looks like just some magic number to indicate "hello". Why not just send "hello" and get it over with? Why pick those specific bytes?

I really thought it didn't matter, just a sequence of bytes for a "hello". Instead of converting hello into bytes at runtime why not just have a sequence of bytes?

 

Client.srcFile, .port, .address. Why are these public?

Good point, I don't worry about encapsulation that much (I would if I was developing a public library). I should start thinking about what proper accessibility I should mark when I am programming though.

 

Thanks for the reply again :)

0

Share this post


Link to post
Share on other sites

And yes, this was of course learning exercise for me. I never developed a file-transfer program before, decided to have a stab at it. Using a library would've defeated the whole purpose. My intention was not to re-invent the wheel.

Edited by Xanather
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0