patterncsharpMinor
Multi threading with webrequests/responses
Viewed 0 times
multiwithresponsesthreadingwebrequests
Problem
I have been working with multi threading and would like some feedback on the code, i generally suck at class design/OO which i am trying to improve. Im sure there is a better way of making it neater and maximizing readability.
```
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
namespace XMLSender
{
class Program
{
private static string serverUrl;
static void Main(string[] args)
{
Console.WriteLine("Please enter the URL to send the XML File");
serverUrl = Console.ReadLine();
List threads = new List();
List names = new List();
string fileName = "";
do
{
Console.WriteLine("Please enter the XML File you Wish to send, To start simulation type 'start'");
fileName = Console.ReadLine();
if (fileName != "start")
{
Thread t = new Thread(new ParameterizedThreadStart(send));
threads.Add(t);
names.Add(fileName);
}
}
while (fileName != "start");
for (int i = 0; i < threads.Count; i++)
{
var t = threads[i];
var name = names[i];
t.Start(name);
}
foreach (Thread t in threads)
{
t.Join();
}
}
static private void send(object data)
{
try
{
//Set the connection limit of HTTP
System.Net.ServicePointManager.DefaultConnectionLimit = 10000;
//ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(http://www.contoso.com/
```
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
namespace XMLSender
{
class Program
{
private static string serverUrl;
static void Main(string[] args)
{
Console.WriteLine("Please enter the URL to send the XML File");
serverUrl = Console.ReadLine();
List threads = new List();
List names = new List();
string fileName = "";
do
{
Console.WriteLine("Please enter the XML File you Wish to send, To start simulation type 'start'");
fileName = Console.ReadLine();
if (fileName != "start")
{
Thread t = new Thread(new ParameterizedThreadStart(send));
threads.Add(t);
names.Add(fileName);
}
}
while (fileName != "start");
for (int i = 0; i < threads.Count; i++)
{
var t = threads[i];
var name = names[i];
t.Start(name);
}
foreach (Thread t in threads)
{
t.Join();
}
}
static private void send(object data)
{
try
{
//Set the connection limit of HTTP
System.Net.ServicePointManager.DefaultConnectionLimit = 10000;
//ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(http://www.contoso.com/
Solution
You say you suck at class design but there's only one class in your code and everything is very "linear". I'm no expert anyway, but I'll try to help.
First of all you should separate the
On the
If you have many variables it might be better to declare this at the start of the function, but it isn't always necessary. In my opinion on the top you should put the variables that will be used many times or that are important.
I would do this with an
Also you're doing the
It's simpler like this:
After this you iterate thorugh the items 2 times, I will go on that later.
Now we can create a class that does the "heavy work". Let's ask ourselves: what do we want this class to do? And what does it need to do that?
Well, now you have the class functions and its params.
We want the class to send data on, it needs the "XML file path", and we want to do it on a thread.
We create a local class variable that's asked when we create the class and that will be accesed later:
Sometimes, using Threads it's nasty and can cause problems and performance issues (if using many of them, so I like using
If we only have one line of code using
Then on the
On the first line we see a
Things that could throw and exception are
Is it necessary to do this every time? Why not take this to the main function? Since it's only executed once.
Then the bytes are read, to reduce code lines we can do this:
Next the HttpResquest is created, if we take this to another function (ideally each function should do only 1 thing), the code will be easier to read and if something has to be change less problems will occur.
We take the cast to the end of the function, since it's not necessary to do it earlier.
Then we want to write the request. If you use
Then the function receives the response, we can take that to a different function too, like this:
In this function we send Data and then we get the response, so the mame must be changed to something like
Now we go back to the top, and we see that we change some things that we had written before, like instead of using a
```
using System;
using System.Collections.Generic;
using System.IO;
using System.Net;
using System.Threading.Tasks;
namespace test
{
internal class Program
{
private static void Main(string[] args)
{
ServicePointManager.DefaultConnectionLimit = 10000;
string serverURL;
List fileNames = new List();
Console.WriteLine("Please enter the URL to send the XML File");
serverURL = C
First of all you should separate the
Main() function from the code you're executing (unlesss the code is short), this will improve readbility at it will be easier to update the code. To do this you can create a class where the main work will happen.On the
Main() function:serverUrl = Console.ReadLine();If you have many variables it might be better to declare this at the start of the function, but it isn't always necessary. In my opinion on the top you should put the variables that will be used many times or that are important.
while (fileName != "start");I would do this with an
if but this works fine. What I would also do is to save all the inputs in a list, either a List<> or a string[].Also you're doing the
"start" checking twice, one on the loop, and one on the code inside the loop. It's simpler like this:
List fileNames = new List();
while (currentLine != "start")
fileNames.Add(currentLine);After this you iterate thorugh the items 2 times, I will go on that later.
Now we can create a class that does the "heavy work". Let's ask ourselves: what do we want this class to do? And what does it need to do that?
Well, now you have the class functions and its params.
We want the class to send data on, it needs the "XML file path", and we want to do it on a thread.
class TestClass
{
private string xmlPath;
public TestClass(string _xmlPath)
{
xmlPath = _xmlPath;
}
public void SendDataAsync(object data)
{
}
public void SendData(object data)
{
}
}We create a local class variable that's asked when we create the class and that will be accesed later:
xmlPath.Sometimes, using Threads it's nasty and can cause problems and performance issues (if using many of them, so I like using
Tasks. This is what will be happening on the SendDataAsyncFunction:public void SendDataAsync(object data)
=> Task.Run(() => SendData(data));If we only have one line of code using
=> makes it simpler than using brackets.Then on the
SendData function: On the first line we see a
try block, then is it really necessary? What could throw an exception? Things that could throw and exception are
File.ReadAllBytes , request.Write and the response, in this case it's ok to have a try block on the whole function, but normally you shouldn't do that. System.Net.ServicePointManager.DefaultConnectionLimit = 10000;Is it necessary to do this every time? Why not take this to the main function? Since it's only executed once.
Then the bytes are read, to reduce code lines we can do this:
byte[] bytes = File.ReadAllBytes(xmlPath);Next the HttpResquest is created, if we take this to another function (ideally each function should do only 1 thing), the code will be easier to read and if something has to be change less problems will occur.
private HttpWebRequest CreateRequest(int length)
{
WebRequest request = WebRequest.Create(ATMSimulator.Properties.Settings.Default.KTHBaseUri);
request.ContentType = "text/xml; encoding='utf-8'";
request.ContentLength = length;
request.Method = "POST";
return (HttpWebRequest)request;
}We take the cast to the end of the function, since it's not necessary to do it earlier.
Then we want to write the request. If you use
using the stream will be closed itelf. We can change the code to this:using (Stream requestStream = request.GetRequestStream())
requestStream.Write(bytes, 0, bytes.Length);Then the function receives the response, we can take that to a different function too, like this:
private string ReceiveResponse(HttpWebRequest request)
{
using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
return new StreamReader(response.GetResponseStream()).ReadToEnd();
}In this function we send Data and then we get the response, so the mame must be changed to something like
SendDataAndGetResponse.Now we go back to the top, and we see that we change some things that we had written before, like instead of using a
string[] we can use a List of the class itelf.(I'm tired from explaining everything, so I just post the final code here):```
using System;
using System.Collections.Generic;
using System.IO;
using System.Net;
using System.Threading.Tasks;
namespace test
{
internal class Program
{
private static void Main(string[] args)
{
ServicePointManager.DefaultConnectionLimit = 10000;
string serverURL;
List fileNames = new List();
Console.WriteLine("Please enter the URL to send the XML File");
serverURL = C
Code Snippets
serverUrl = Console.ReadLine();while (fileName != "start");List<string> fileNames = new List<string>();
while (currentLine != "start")
fileNames.Add(currentLine);class TestClass
{
private string xmlPath;
public TestClass(string _xmlPath)
{
xmlPath = _xmlPath;
}
public void SendDataAsync(object data)
{
}
public void SendData(object data)
{
}
}public void SendDataAsync(object data)
=> Task.Run(() => SendData(data));Context
StackExchange Code Review Q#137826, answer score: 7
Revisions (0)
No revisions yet.