Averaging nearby points in an array

Question

The following code takes in an point array and removes all of the nearby points whilst averaging them based on the conditions past in.

The variable _nCornerTolerance is an int that is calculated prior in the class. Typically it is around 5;

private Point[] AverageNearbyPoints(Point[] points, bool averageLeft, bool averageHigh)
{
 List<Point> plAveragedPoints = new List<Point>();
 for (int pi = 0, pc = points.Length; pi < pc; ++pi)
 {
 Point[] paNearbyPoints = points.Where(p => (Math.Abs(p.X - points[pi].X) <= _nCornerTolerance)
 && (Math.Abs(p.Y - points[pi].Y) <= _nCornerTolerance))
 .ToArray();
 if (paNearbyPoints.Length > 1)
 {
 int nX = averageLeft ? (int)Math.Floor(paNearbyPoints.Sum(p => p.X) / (float)paNearbyPoints.Length)
 : (int)Math.Ceiling(paNearbyPoints.Sum(p => p.X) / (float)paNearbyPoints.Length);
 int nY = averageHigh ? (int)Math.Floor(paNearbyPoints.Sum(p => p.Y) / (float)paNearbyPoints.Length)
 : (int)Math.Ceiling(paNearbyPoints.Sum(p => p.Y) / (float)paNearbyPoints.Length);
 if (!plAveragedPoints.Any(p => p.X == nX && p.Y == nY))
 {
 plAveragedPoints.Add(new Point(nX, nY));
 }
 }
 else
 {
 if (!plAveragedPoints.Any(p => p.X == points[pi].X && p.Y == points[pi].Y))
 {
 plAveragedPoints.Add(points[pi]);
 }
 }
 }
 return plAveragedPoints.ToArray();
}

Is there anyway I can rewrite the int nX = ... and int nY = ... parts better so I don't need the conditional?

A full review of this code would also be helpful, although the main part I am asking about is the above.

Answer 1

Naming

• plAveragedPoints
• paNearbyPoints

this variables would be more obvious if you wouldn't use this strange kind of hungarian notation. I figured out the pl means point list and pa point array, but if you need to read this fast you just can't because your brain will try to decypher what pl and pa means.

Does it really matter wether it is a List or an Array ?

---

 if (!plAveragedPoints.Any(p => p.X == nX && p.Y == nY))
 {
 plAveragedPoints.Add(new Point(nX, nY));
 } 

and

 if (!plAveragedPoints.Any(p => p.X == points[pi].X && p.Y == points[pi].Y))
 {
 plAveragedPoints.Add(points[pi]);
 } 

made me think that maybe a HashSet<Point> would be the better datastructure instead of a List<Point. You could simply call the Add() method and forget about the result of it.

---

Instead of using the tenary expression, @Caridoc had shown a different implementation which was a good start.

I would suggest to add another method which takes two Func<double,double> which are determined before calling that overloaded method like so

 private Func<double, double> CreateDesiredFunc(bool useFloor)
 {
 if (useFloor)
 {
 return Math.Floor;
 }
 return Math.Ceiling;
 }

Edit: Like @RickDavin pointed out in the comments this can be cleaned a little bit more by using more idiomatic names for the looping index variable and for the coordinate variables. In addition you should/could use the var type where it is clear from the right hand side of an assignment what type it is.

This boils down to the following

 private Point[] AverageNearbyPoints(Point[] points, bool averageLeft, bool averageHigh)
 {
 return AverageNearbyPoints(points, CreateDesiredFunc(averageLeft), CreateDesiredFunc(averageHigh));
 }
 private Point[] AverageNearbyPoints(Point[] points, Func<double, double> averageLeft, Func<double, double> averageHigh)
 {
 var averagedPoints = new HashSet<Point>();
 int numberOfPoints = points.Length;
 for (int i = 0; i < numberOfPoints ; ++i)
 {
 var nearbyPoints = points.Where(p => (Math.Abs(p.X - points[i].X) <= _nCornerTolerance)
 && (Math.Abs(p.Y - points[i].Y) <= _nCornerTolerance))
 .ToArray();
 var point = points[pi];
 if (nearbyPoints.Length > 1)
 {
 int x = (int)averageLeft(nearbyPoints.Sum(p => p.X) / (float)nearbyPoints.Length);
 int y = (int)averageHigh(nearbyPoints.Sum(p => p.Y) / (float)nearbyPoints.Length);
 point = new Point(x, y);
 }
 averagedPoints.Add(point);
 }
 return averagedPoints.ToArray();
 }

• TheLethalCoder: Very helpful, the naming convention is something from my workplace so I am stuck with it. Once you get used to it, it is easier to tell what a data structure is when you can't see where it is initialised