Program structure to be checked according to OOP

I am building my first application based on the OOP model. The application basically responsible for sending XML Soap requests. Model contains couple classes - Server.cs which contain servers IPs, Query.cs for XML soap texts, Message.cs which will contain list of queries put to specific message to be send, and Sender.cs to take Messages to send requests.

Please check whether my OOP way of preparing that application is good enough and what do you think how I related classes and dealing with object - as well as generic class used in this project.

Is this a good way of programming? Note that this is not finished yet I want to take your suggestion under consideration if I am using OOP well enough.

```
class Program
{
static void Main(string[] args)
{
//Queries belongs to message
var Query1 = new Query() { ID = 1, Message = "First XML query"};
var Query2 = new Query() { ID = 2, Message = "Second XML query" };

//Message contains queries
Message msg1 = new Message();
Utilities.AddToList(Query1);
Utilities.AddToList(Query2);
Utilities.RevertList();

Utilities.RemoveFromList(Query1);

//Message contains servier ip
msg1.Server.ServerIP = "192.168.0.1";

//all below is just for testing - here will be Sender execute method realased instead of below:
foreach (Query item in Utilities.GetExistingList())
{
Console.WriteLine("Query ID : " + item.ID.ToString());
Console.WriteLine("Query Message : " + item.Message.ToString());
Console.WriteLine("Server IP : " + msg1.Server.ServerIP.ToString());
}
Console.ReadLine();
}
}

public class Sender
{
private Message Message;
private List _messageList;

public void GetCompleteSendInfromation()
{
Console.WriteLine("Server IP : " + Message.Server.ServerIP);
Console.ReadLine();
}

public void ExecuteSOAP()
{
// TODO: Execite

Utilities.AddToList(Query1);
Utilities.AddToList(Query2);
Utilities.RevertList();

Utilities.RemoveFromList(Query1);

Please, don't do this. Why have a global list of queries that can be accessed from anywhere? This is what Wikipedia has to say about global variables (your list technically isn't a global variable, but it behaves exactly like one):


They are usually considered bad practice precisely because of their non-locality: a global variable can potentially be modified from anywhere, and any part of the program may depend on it. A global variable therefore has an unlimited potential for creating mutual dependencies, and adding mutual dependencies increases complexity.

Instead, just use a local List in your method and pass it as a parameter or store it in a field of your object.


I had got also other version but I wanted to have static List helper class Utilities, which is why I dropped this below one. Is it better OOP than the above version?

Yes, this code is definitely better. Why would you want to have the list static like this? That's pretty much asking for bugs.

public Server _server;
public Server Server
{
    get { return _server; }
    set { _server = value; }
}

You don't need to be this verbose, just use an auto-property:

public Server Server { get; set; }

"Message contains: " + Utilities.List.Count.ToString()
    + " queries and 1 Server IP address"

You don't need that ToString() there, that's called automatically. Also I think it's better to use string.Format() in cases like this (it makes especially dealing with punctuation and spaces much clearer):

string.Format(
    "Message contains: {0} queries and 1 Server IP address", Utilities.List.Count);

StackExchange Code Review Q#58937, answer score: 3