PlayerConnection class for Gaming Emulator

I have programmed a Gaming Emulator in C# and I have a class to handle all the connection side of things for the player, here we continuously listen for packets (each packet has a ID like a number, from 1 - 300), I just wondered is there any way I could speed it up or make it better performance wise? It seems a bit messy at the moment..

```
using Faze.Other.Util;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;

namespace Faze.Base.Game.Habbo.Players
{
class PlayerConnection
{
private readonly Socket connectionSocket;
private bool packetsActive;
private readonly byte[] connectionBuffer;
private bool isPlayerDisconnecting;

public PlayerConnection(Socket playerSocket)
{
connectionSocket = playerSocket;
connectionSocket.SendBufferSize = 8000;
connectionBuffer = new byte[8000];
}

public Socket PlayersSocket
{
get
{
return connectionSocket;
}
}

private void ListenForPackets()
{
if (packetsActive)
return;

packetsActive = true;

try
{
connectionSocket.BeginReceive(connectionBuffer, 0, connectionBuffer.Length, SocketFlags.None, OnPacketReceived, connectionSocket);
}
catch
{
CloseSocket();
}
}

private void CloseSocket()
{
try
{
if (!packetsActive)
return;

packetsActive = false;

if (SocketConnected(connectionSocket))
{
connectionSocket.Shutdown(SocketShutdown.Both);
connectionSocket.Close();
}

connectionSocket.Dispose();
}

Private functions & fields aren't called/modified within the class:

-
ListenForPackets is private and isn't called by anything within the class

-
packetsActive is private and isn't written to by anything within the class

(so they can only be accessed through reflection, which is not a normal way for classes to interact)

Note that the private accessibility level in C# is intended to mean that the entry can only be accessed within the same type, i.e., within PlayerConnection only.

The containing class is not intended to have any special access rights at all.

For a demonstration see this code below:

Or try it here: https://ideone.com/zJGFE0

using System;

public class MyClassContainer
{
    private MyClass MyClass_Instance;
    
    public void DemonstrateAccessLevels()
    {
        MyClass_Instance = new MyClass();
        
        MyClass_Instance.SomePublicMethod();
        
        MyClass_Instance.SomeInternalMethod();
        
        // we can't access protected or private methods of MyClass from MyClassContainer
        //
        //error CS0122: `MyClass.SomeProtectedMethod()' is inaccessible due to its protection level
        //MyClass_Instance.SomeProtectedMethod();
        
        //error CS0122: `MyClass.SomePrivateMethod()' is inaccessible due to its protection level
        //MyClass_Instance.SomePrivateMethod();
    }
}

public class MyClass
{
    private void SomePrivateMethod()
    {
        Console.WriteLine("Some Private Method");
    }
    
    protected void SomeProtectedMethod()
    {
        Console.WriteLine("Some Protected Method");
    }
    
    internal void SomeInternalMethod()
    {
        Console.WriteLine("Some Internal Method");
    }
    
    public void SomePublicMethod()
    {
        Console.WriteLine("Some Public Method");
    }
}

public class Test
{
    public static void Main()
    {
        MyClassContainer testContainer = new MyClassContainer();
        
        testContainer.DemonstrateAccessLevels();
    }
}

The Accessibility Levels on MSDN article gives you your access rights options in C#, I recommend designing within these access rights and not trying to work around them with reflection.

Please note that "containing class" refers to the class being defined, NOT the class containing an instance of the class we're defining.

-
public: Access is not restricted.

-
protected: Access is limited to the containing class or types derived from the containing class.

-
internal: Access is limited to the current assembly. [i.e., current .exe or .dll]

-
protected internal: Access is limited to the current assembly or types derived from the containing class.

-
private: Access is limited to the containing type.

It would be clearer to add getters/setters and public methods to access your class's state instead of using reflection to get at private methods. Using reflection to get at private methods is kind of deceptive and people who read your code might not expect code outside of the class to be modifying those fields / calling those methods.

If you put a private method in a class, and never call it within the class, it looks like dead code. Even worse, it breaks encapsulation, if any class can modify your class's private fields, your class doesn't really offer any protection.

Note that in addition to this, invoking methods using reflection can be very slow, for more information see this question: https://stackoverflow.com/questions/10313979/methodinfo-invoke-performance-issue

Encapsulation anti-pattern:

The PlayerSocket getter allows other classes to run functions to modify the socket. Your PlayerSocket getter is in my opinion worse than making the player's Socket a public field.

If your intention is to have this class encapsulate your socket, this won't do it, keep in mind external classes will still be able to call stuff like Close() without PlayerConnection even knowing about it, making the CloseSocket() method on PlayerConnection an optional method, and kind of deceptive.

My recommendations for this part of the code depend on what you want to accomplish.

-
If you want PlayerConnection to encapsulate the Socket:
I would recommend getting rid of the Getter property and add public methods to PlayerConnection that provide access to only the methods on Socket that you want external classes to access (e.g., maybe everything except Socket.Close()). That way you can better control access to the socket.

-
If you don't want PlayerConnection to encapsulate the Socket, and you're okay with other classes modifying the Socket:
I would recommend possibly moving all of this code into the Player class, if feels too cumbersome to restrict access to the socket this may not need its own distinct class.

If the Player class is already very big, I'd recommend posting a new code review with both the Player class and the PlayerConnection class, with the access modifiers fixed; we'd probably be able to give more advice

StackExchange Code Review Q#134865, answer score: 8