Jump to content

  • Log In with Google      Sign In   
  • Create Account

Quality of my code


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
7 replies to this topic

#1 Xanather   Members   -  Reputation: 712

Like
1Likes
Like

Posted 11 January 2013 - 04:03 AM

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.



Sponsor:

#2 KnolanCross   Members   -  Reputation: 1369

Like
2Likes
Like

Posted 11 January 2013 - 09:11 AM

The basics should be divide your code into methods and use private variables.


Currently working on a scene editor for ORX (http://orx-project.org), using kivy (http://kivy.org).


#3 Xanather   Members   -  Reputation: 712

Like
0Likes
Like

Posted 11 January 2013 - 11:41 PM

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?



#4 magicstix   Members   -  Reputation: 191

Like
0Likes
Like

Posted 12 January 2013 - 12:14 AM

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. >:]



#5 DarkRonin   Members   -  Reputation: 616

Like
0Likes
Like

Posted 31 January 2013 - 05:36 PM

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



#6 frob   Moderators   -  Reputation: 22812

Like
3Likes
Like

Posted 01 February 2013 - 12:53 AM

Putting my code review hat on and reading through the code in the order you posted it.

 

Program.Main().

Make a function to read args first.

Make the output of your title and text blurb optional.  In the unix mentality you wouldn't show them at all unless the --version parameter was used.

Get rid of the "Press any key to close" message, or at least make it not there by default, unless an --interactive parameter was used.

Move the error messages to a separate function.  Instead of telling them the correct options, ask them to use the --help parameter.

 

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);

 

 

Client.clientVerify array. I can see what you are trying to do with it, but WHY are you trying to use it?  I cannot see any benefit, more on that later.

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

Client.Start() method.

What's with all the colorful output?  This function has about 35 lines trying to output something that looks nice, when really all you need is a hard failure.  You can add an --interactive or --verbose flag that shows the pretty cyan if you think that is somehow helpful to your users.  Personally I find it a distraction.  Either succeed in silence or fail noisily; with bright red if you must use a color.

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.

Cleaning that up brings your 90 line function down to this:

 

            address = args[1];
            srcFile = args[2];
            port = Server.defaultPort;
            if(args.Length>2) {
                int tempVal;
                port = int.TryParse(args[3], out tempVal) ? tempVal : Server.defaultPort;
            }
            if(ValidateParams())
                ProcessConnection();

Much easier to read than 90 lines of console font coloring.

 

 

Client.ProcessConnection.

First thing I did when reviewing this one was to remove another 30 lines of font coloring and verbose output.  If you absolutely must have them, make sure they are off by default.  If you want a --verbose flag or --interactive flag, use that.

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?  Do they buy you anything?

Rather than converting everything into byte arrays, use a StreamWriter object.  That's why they exist.

Just those changes alone drop your function down to around 70 lines of code.  I'd reduce it further, but you should get the idea at this point.

 

 

The rest of the code suffers from the same problems.  

 

Just running over the rest of it briefly I dropped it to 1/3 of its original size.  That makes it easier to understand.  And that makes it less prone to bugs, and easier to fix those bugs when they are discovered.

 

 

 

Out of curiosity, why not use any of the bajillion existing file transfer tools out there?  Is this a learning experiment?  If not, I'd recommend going with SFTP.  It is a long-established secure protocol that can be called directly as a standalone executable or with a wrapper in your code.  Or if you want to go with .NET, use the Background Intellegent Transfer Service (BITS).  Using either of those directly is still a tiny fraction of the code you have written above.

 

 

 

 

If you mean this as a learning experience about networking, it looks like you have done all the basics.  As far as a learning experiment goes it appears to be functional, and that is often enough by itself.  

 

Putting myself in a teaching role, I would put it in this type of category:  "You just wrote your own linked list class.  Congratulations, you understand linked lists.  Now that you know how they work, never write a linked list again if you can help it, you should always use the built-in classes."  You just learned the basics of how to transfer data.  Now that you know how and why it works, you have learned all you need.  From now on use a well-written, established, debugged, optimized library rather than wasting your time re-inventing the wheel.

 

To "rate the quality" of your code as you requested, it is clearly a beginner's code.  It would not survive a code review in a professional environment (one of us would sit down with the junior developer and help him fix the issues in an attempt to better his skills).  


Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I write about assorted stuff.


#7 Xanather   Members   -  Reputation: 712

Like
0Likes
Like

Posted 02 February 2013 - 10:58 AM

@ 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 :)



#8 Xanather   Members   -  Reputation: 712

Like
0Likes
Like

Posted 02 February 2013 - 11:00 AM

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, 02 February 2013 - 11:02 AM.





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS