I have a class which is responsible for checking whether a website is up or down. I've made it to run asynchronously, but I would like critique if this is a good/bad way or if there is another better way to do it.
class UrlChecker {
private readonly IValidationCondition _condition;
private readonly Dictionary<string, UrlStatus> _status = new Dictionary<string, UrlStatus>();
private readonly object _lock = new object();
public UrlChecker(IValidationCondition condition) {
_condition = condition;
}
public void CheckRange(IEnumerable<string> urls, Action<Dictionary<string, UrlStatus>> callback) {
var options = new ParallelOptions {MaxDegreeOfParallelism = 5};
Parallel.ForEach(urls, options, Check);
callback(_status);
}
private void Check(string url) {
Console.WriteLine("Checking " + url);
var req = (HttpWebRequest) WebRequest.Create(url);
req.Timeout = 10 * 10000; // 10 seconds
HttpWebResponse resp;
try {
resp = (HttpWebResponse)req.GetResponse();
}
catch(WebException ex) {
// We got an exception, consider it as down
lock (_lock)
_status.Add(url, UrlStatus.Down);
return;
}
if(resp.StatusCode != HttpStatusCode.OK) {
lock (_lock)
_status.Add(url, UrlStatus.Down);
return;
}
using(var reader = new StreamReader(resp.GetResponseStream())) {
// Check for empty response
var html = reader.ReadToEnd();
if(string.IsNullOrEmpty(html)) {
lock(_lock) {
_status.Add(url, UrlStatus.Down);
}
}
// Validate against condition
if(!_condition.IsValid(html)) {
lock(_lock) {
_status.Add(url, UrlStatus.Down);
}
return;
}
}
// We reached the end without problems, it's a valid url
lock(_lock) {
_status.Add(url, UrlStatus.OK);
}
}
}
It's called like so:
checker.CheckRange(urls, status => {
if(status.Any(x => x.Value == UrlStatus.Down))
EmailFailing(message);
});
The second parameter is obviously a callback that's invoked when all checks are done.
Am I locking correctly?
Is this an acceptable way of doing it? Checking with Fiddler proves that it's working correctly, but is there a better way?
