patterncsharpMinor
Asynchronously loading a point in chart on drag
Viewed 0 times
pointchartdragloadingasynchronously
Problem
I have some code that asynchronously loads some point in a chart when a users drag it:
This function calls something like:
```
UpdateHistoryAsyc UHA = new UpdateHistoryAsyc(UpdateHistoryAndReSizeWhenDrag);
IAsyncResult r
private void UpdateHistoryAndReSizeWhenDrag(string param, long duration)
{
lock (Lock)
{
if (Boxes.Count() > 0)
{
BoxStruct temp = Boxes.Where(key => key.id == param).FirstOrDefault();
if (temp != null)
{
int index = Boxes.IndexOf(Boxes.Where(key => key.id == param).First());
if (index >= 0)
{
double ChartLastPoint = Boxes[index].Series.Points[0].X;
double LastShownPoint = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();
double PMActualMinimumY = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();
double PMActualMaximumY = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMaximum).Max();
if (LastShownPoint LoadedPoints = LoadPointsRange(param, StartTime, EndTime).Points;
Boxes[index].Series.Points.InsertRange(0, LoadedPoints);
}
if (Boxes.Count > 0 & index key.X > PMActualMinimumY && key.X 0)
{
double max = temp1.Select(p => p.Y).Max();
double min = temp1.Select(p => p.Y).Min();
Boxes.Where(k => k.id == param).First().PlotModel.Axes[1].Reset();
Boxes.Where(k => k.id == param).First().PlotModel.Axes[1].Zoom(min, max);
}
}
}
}
}
}
}This function calls something like:
```
UpdateHistoryAsyc UHA = new UpdateHistoryAsyc(UpdateHistoryAndReSizeWhenDrag);
IAsyncResult r
Solution
You don't follow capitalization conventions:
I suspect
Use proper variable names, not
Your code suffers from the Arrow anti-pattern. You can easily reduce some of the nesting by
I don't know what
I don't see why you're using
I also don't see the point of this check:
I find it a bit odd that you carefully check that Boxes contains a value you can work with, but then never check if that object's
I see the magic number 10800 appear twice. This should be stored in a constant and have a descriptive name.
Why do you retrieve the same value twice, yet store it in different variables?
Moreover, since you need to use the result of
I'm a bit worried to see a method named
ChartLastPoint, LastShownPoint, PMActualMinimumY, PMActualMaximumY, StartTime, EndTime, LoadedPoints should all be camelCase.I suspect
Lock must be camelCase as well, since I doubt it is a public property.Use proper variable names, not
temp or temp1. Same problem with param: it doesn't tell me anything, only through the code I know it's some kind of ID. Your code suffers from the Arrow anti-pattern. You can easily reduce some of the nesting by
returning earlier:private void UpdateHistoryAndReSizeWhenDrag(string param, long duration)
{
lock (Lock)
{
if (Boxes.Count() key.id == param).FirstOrDefault();
if (temp == null)
{
return;
}
int index = Boxes.IndexOf(Boxes.Where(key => key.id == param).First());
if (index < 0)
{
return;
}
/* rest of your code */
}
}I don't know what
Boxes is, but I suspect an ICollection or an IList of sorts, so why then use Boxes.Count() when you can use Boxes.Count (as you do later on in your code)?I don't see why you're using
Boxes[index], since the temp contains the BoxStruct you need to work with. Later on again you do Boxes.Where(k => k.id == param).First() twice, even though you could simply use temp.I also don't see the point of this check:
if (Boxes.Count > 0 & index 0).I find it a bit odd that you carefully check that Boxes contains a value you can work with, but then never check if that object's
Series isn't null, and if Series's Points isn't null etc. Also, you don't do anything if you can't find an appropriate BoxStruct -- shouldn't you inform the user?I see the magic number 10800 appear twice. This should be stored in a constant and have a descriptive name.
Why do you retrieve the same value twice, yet store it in different variables?
double LastShownPoint = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();
double PMActualMinimumY = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();Moreover, since you need to use the result of
from x in Boxes where x.id == param select x.PlotModel.Axes[0] three times, store it in a variable and use that variable.PMActualMinimumY and PMActualMaximumY both end in Y, yet you compare them to a value named X.I'm a bit worried to see a method named
LoadPointsRange which seemingly returns some kind of object, from which you then take its property Points.Code Snippets
private void UpdateHistoryAndReSizeWhenDrag(string param, long duration)
{
lock (Lock)
{
if (Boxes.Count() < 1)
{
return;
}
BoxStruct temp = Boxes.Where(key => key.id == param).FirstOrDefault();
if (temp == null)
{
return;
}
int index = Boxes.IndexOf(Boxes.Where(key => key.id == param).First());
if (index < 0)
{
return;
}
/* rest of your code */
}
}double LastShownPoint = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();
double PMActualMinimumY = (from x in Boxes where x.id == param select x.PlotModel.Axes[0].ActualMinimum).Min();Context
StackExchange Code Review Q#93319, answer score: 2
Revisions (0)
No revisions yet.