I started to have HUGE doubts regarding my code and I need some advice from more experienced programmers and architects.
On the button click, the application runs a method, that is running a ScrapJockeys
with parameters:
if (UpdateJockeysPl) await ScrapJockeys(JPlFrom, JPlTo + 1, "jockeysPl"); //1 - 1049
Purpose of this is to scrap some data from web service and parse it with a model:
public class LoadedJockey
{
public string Name { get; set; }
public string Link { get; set; }
public List<JockeyRaceDetails> AllRaces { get; set; }
}
When the data is parsed, object is added to ObeservableCollection<LoadedJockey>
and the collection is saved. Thanks to this, and based on the data the application can do computation of Jockey performace in the past.
And now more about the method and problems with the method:
I have a method, that is triggering a for
loop, repeating between 20K - 150K times. Inside the loop I need to call a service method, that execution of the method takes a lot of time. Also I wanted to have ability of canellation of the loop and everyting what is going on inside of the loop/method.
Right now I have a method with list of tasks, and inside of the loop is trigerred a Task.Run
. And inside of each task I am calling awaited service method. Also each task has assigned cancellation token, like IN THE EXAMPE:
public async Task ScrapJockeys(int startIndex, int stopIndex, string dataType)
{
//init values and controls
CommandStartedControlsSetup("UpdateDataCommand");
List<Task> tasks = new List<Task>();
int loopCounter = 0;
ProgressBarTick("Looking for jockeys", loopCounter, stopIndex, startIndex);
for (int i = startIndex; i < stopIndex; i++)
{
if (TaskCancellation == true)
{
break;
}
int j = i;
Task task = Task.Run(async () =>
{
LoadedJockey jockey = new LoadedJockey();
CancellationToken.ThrowIfCancellationRequested();
if (dataType == "jockeysPl") jockey = await _scrapServices.ScrapSingleJockeyPlAsync(j);
if (dataType == "jockeysCz") jockey = await _scrapServices.ScrapSingleJockeyCzAsync(j);
if (jockey.Name != null)
{
lock (((ICollection)Jockeys).SyncRoot)
{
//if objects are already in the List
if (Jockeys.Any(h => h.Name.ToLower() == jockey.Name.ToLower()))
{
LoadedJockey doubledJockey = Jockeys.Where(h => h.Name.ToLower() == jockey.Name.ToLower()).FirstOrDefault();
Jockeys.Remove(doubledJockey);
MergeJockeysData(doubledJockey, jockey);
}
else
{
Jockeys.Add(jockey);
}
}
}
loopCounter++;
//saves all every 1000 records, just in case
if (loopCounter % 1000 == 0)
{
await _dataServices.SaveAllJockeysAsync(Jockeys.ToList());
}
ProgressBarTick("Looking for jockeys", loopCounter, stopIndex, startIndex);
}, TokenSource.Token);
tasks.Add(task);
}
try
{
await Task.WhenAll(tasks);
}
catch (OperationCanceledException)
{
//
}
finally
{
await _dataServices.SaveAllJockeysAsync(Jockeys.ToList()); //saves everything to JSON file
AllControlsEnabled = true;
CommandCompletedControlsSetup();
VisibilityCancellingMsg = Visibility.Collapsed;
}
}
So my question is, is everything fine with my code? According to THIS ARTICLE:
Many async newbies start off by trying to treat asynchronous tasks the same as parallel (TPL) tasks, and this is a major misstep.
What should I use then?
And according to THIS ARTICLE:
On a busy server, this kind of implementation can kill scalability.
So how am I supposed to do it?
Please be noted, that service interface method signature is Task<LoadedJockey> ScrapSingleJockeyPlAsync(int index);
And also i am not 100% sure that I am using Task.Run
correctly within my service class. The methods inside are wrapping the code inside await Task.Run(() =>
, like IN THE EXAMPLE:
public async Task<LoadedJockey> ScrapSingleJockeyPlAsync(int index)
{
LoadedJockey jockey = new LoadedJockey();
await Task.Run(() =>
{
int n;
List<JockeyRaceDetails> allRaces = new List<JockeyRaceDetails>();
for (int year = DateTime.Now.Year; year > 2013; year--)
{
StringBuilder sb = new StringBuilder();
sb.Append("https://koniewyscigowe.pl/dzokej?d=");
sb.Append(index);
sb.Append("&sezon=");
sb.Append(year);
sb.Append("#wyniki_koni");
string link = sb.ToString();
HtmlAgilityPack.HtmlWeb web = new HtmlAgilityPack.HtmlWeb();
HtmlAgilityPack.HtmlDocument doc = web.Load(link);
//gets the name of the jockey
if (jockey.Name == null)
{
try
{
HtmlNode singleNode = doc.DocumentNode.SelectSingleNode("/html/body/main/section[1]/div[1]/h3");
string racersName = singleNode.OuterHtml.ToString();
if (racersName.Contains("Jeździec") && racersName.Length > 65)
{
racersName = racersName.Split('>')[1].Split(new string[] { " - " }, StringSplitOptions.None)[1].Split('<')[0].Trim(' ');
if (racersName.Contains(" "))
{
char letter = racersName[0];
racersName = racersName.Split(' ')[1].Trim(' ');
racersName = letter + ". " + racersName;
}
racersName = MakeTitleCase(racersName);
jockey.Name = racersName; //jockeys name
}
else
{
racersName = "";
}
}
catch (Exception e)
{
}
}
//scrap races
HtmlNode[] tableRow = doc.DocumentNode.SelectNodes("//*[@id=\"wykaz_list\"]/tbody/tr").ToArray();
if (tableRow != null && tableRow.Length > 0)
{
foreach (var row in tableRow)
{
string stringTableRow = row.OuterHtml.ToString();
//all races
//if row not contains 'brak startow' and contains m (meters)
if (!stringTableRow.Contains("Brak danych") && (stringTableRow.Contains(" m") || stringTableRow.Contains(" m")))
{
try
{
JockeyRaceDetails race = new JockeyRaceDetails();
string raceDate = stringTableRow.Split('>')[3].Split('<')[0].Trim(' ');
string raceDistance = stringTableRow.Split('>')[8].Split(' ')[0].Trim(' ');
string horsesName = stringTableRow.Split('>')[10].Split('<')[0].Trim(' ');
horsesName = MakeTitleCase(horsesName);
string raceScore = stringTableRow.Split('>')[12].Split('<')[0].Trim(' ');
string racePlace = raceScore.Split('/')[0].Trim(' ');
string raceCompetitors = raceScore.Split('/')[1].Trim(' ');
race = ParseJockeyRaceData(raceDate,
raceDistance,
horsesName,
raceScore,
racePlace,
raceCompetitors);
allRaces.Add(race);
}
catch (Exception e)
{
}
}
}
}
}
jockey.AllRaces = allRaces; //jockeys races between 2015-2019
jockey.Link = "https://koniewyscigowe.pl/dzokej?d=" + index; //racers link
if (jockey.AllRaces.Count == 0)
{
jockey.Name = null;
}
});
return jockey;
}
As far as I understand from the articles, this is kind of anti-pattern. But I am confused a bit. Basing on THIS SO REPLY, it should be fine...? If not, how to replace it?
j
I was passing properID
to the service. It is based on similar case: stackoverflow.com/a/33275940/11027921 \$\endgroup\$Task.Run
. Mr Stephen Cleary is suggesting in his articles, that I am doing it wrong. But in his SO reply he said nothing about that. So I am confused now. \$\endgroup\$