Which declaration is better?

Question

Hello,

I'm going to change my C# style in programming(I've been using 'public static' for variables,methods - everything).

My question:

public class WinSock
{
    public Socket sock;
    public byte[] data;
    .....
}

var data = new byte[2058];
data = WinSock.data;

or this one:

private class WinSock
{
    private Socket sock;
    private byte[] data;
    .....
    public byte[] getdata()
    {
        get {return data;}
    }
}

WinSock ws = new WinSock();
var data = new byte[2058];
data = ws.getdata();

In both cases the data and sock variables can be accessed from other classes.

Which of the two declarations is better?

Answer 1

Generally, making fields public is never a good idea.

Since getdata is a property, it should be called Data.

private class WinSock
{
    private Socket sock;
    private byte[] data;
    .....
    public byte[] Data
    {
        get { return data; }
    }
}

The public interface, consisting of public properties and methods, should ensure that the state (the fields) is never inconsistent. The class is responsible for the fields. it should not allow anyone to write directly into it. This is what encapsulation is about.

Answer 2

The second. It is called "Encapsulation". You hide and protect your data from the outside world.

Answer 3

From pure "declaration" side, second is slightly better. However, from design perspective (and from what can be seen), this is no good. Specifically, returning a reference to an array which is a private field in a class, is a big no-no. Consider:

private class WinSock
{
    private Socket sock;
    private byte[] data;
    .....

    // Read at most length bytes of data to buffer starting at offset
    // and return the number of bytes read
    public int Read(byte[] buffer, int offset, int length)
    {
        // ...
    }
}

WinSock ws = new WinSock();
var data = new byte[2058];
int bytesRead = ws.Read(data, 0, data.Length);

Answer 4

How about:

public Socket Sock { get; private set; }
public byte[] Date { get; private set; }

Automatically implemented properties are the way to go if you getters and setters do not require any validation logic. This provides encapsulation and allows you later to change the implementation to a manually implemented property, if needed, without forcing recompiles of .dll's that reference yours.

Answer 5

• A class can only be private if it is nested - and then access is limited to the outer classes. internal would be more common (and is the default for non-nested classes)
• Don't use public fields (public Socket sock;)
• Do consider automatically implemented properties (public Socket Socket {get; private set;}, for example)
• Decide wheter getdata is a method or a property ;-p

Answer 6

Neither, instead of using a GetMethod(), you should wrap your private fields in properties.

public class WinSock
{
    private Socket sock;
    private byte[] data;

    public byte[] Data
    {
       get { return data; }
       set { data = value; }
    }
}

Answer 7

The second one is closer to what's generally considered to be better. In Object Oriented Programming we encapsulate the variables in the classes and only expose as little as needed outside the class.

Name your properties with an initial capital letter, and as a data member rather than a method, i.e. Data rather than getdata.

public class WinSock {

    private Socket _socket;
    private byte[] _data;

    public Socket Socket {
       get { return _socket; }
    }

    public byte[] Data {
        get { return _data; }
    }

}

(One way to name the private member variables is to use an underscore. There are some other conventions commonly used, but the important thing is really to pick one and stick to it.)

In C# 3 there is also a shortcut syntax for properties:

public class WinSock {

    public Socket Socket { get; private set; }

    public byte[] Data { get; private set; }

}

Answer 8

I would also vouch for the second code bit, with the amendment that you wrap your fields into properties and naming them properly. So, you would name your property "Data" rather than "getdata".

Answer 9

A few additional points:

1. If you plan on unit testing the code that uses the WinSock class you should consider making the class public instead of internal and consider making the methods virtual. This type of class often needs to be mocked or stubbed to return different data to the caller, so that different paths in the callers code can be executed.
2. You should consider how objects of this class will be constructed. For example, will you pass in the Socket, or will you pass in some data to the class so that it can create the Socket? Your goal should be to pass in whatever data is required to create the object in a useful state.
3. You should consider how you will test this class. In particular, how will this class obtain any other classes it needs to use to accomplish its task. For that you should search on Dependency Injection. That will start you down a useful path.