Etydhv

I got a C# class:

public class Agent 

 public string AgentId set; get; 
 public double Longitude set; get; 
 public double Latitude set; get; 

There are several agents in several different locations. During the program's lifetime, they can receive a call to another (latitude, longitude) point which is unknown during runtime. I got an API to calculate the distance in meters between their location and the given point. What I need eventually is to find the 5 closest agents to the given point.

What I did in order to achieve this is adding another class:

public class AgentDistance : Agent, IComparable<AgentDistance>


 public AgentDistance(Agent agent)
 
 if(agent == null)
 
 throw new Exception("Can't initalize agent with distance since agent is null");
 
 this.AgentId = agent.AgentId;
 this.Latitude = agent.Latitude;
 this.Longitude = agent.Longitude;
 

 public double Distance set; get; 

 public int CompareTo(AgentDistance other)
 
 if(other == null)
 
 return 1;
 
 return Distance.CompareTo(other.Distance);
 

And the usage:

var agents = db.GetAgents();
if(agents == null || agents.Count == 0)

 Console.WriteLine("No Results");

List<AgentDistance> agentsWithDistance = new List<AgentDistance>();
foreach(Agent agent in agents)

 double res = ws.GetDistanceMeters(agent.Latitude, agent.Longitude, customerLatitude, customerLongitude);
 agentsWithDistance.Add(new AgentDistance(agent) Distance = res );

agentsWithDistance.Sort();
for(int i = 0; i < 5; i++)

 Console.WriteLine(string.Format("agent_id: 0 distance: 1 m", agentsWithDistance[i].AgentId, agentsWithDistance[i].Distance));

It works, but is there a more elegant way to do it? I'm not sure if adding another class might be a bit redundant since all it does is just adding a property for sorting, but adding the distance property to the Agent class, doesn't make so much sense.

var agents = db.GetAgents();
if(agents == null || agents.Count == 0)

 Console.WriteLine("No Results");

You should exit your application/code block when you detect an invalid state. Here even if "No Results" is printed, the app will still run into a NullReferenceException or IndexOutOfRangeException in the next few steps. Also, talking of IndexOutOfRangeException, there is no check against if there is at least 5 agents in your db.

Instead of creating an IComparable, you can just use linq to sort directly. But, since in your case, you also need to print out the distance (the value used for sorting), we will need to create an anonymous class to hold it:

var agents = db.GetAgents();
if(agents == null || agents.Count == 0)

 Console.WriteLine("No Results");
 return;


var nearestAgents = agents
 .Select(x => new 
 
 x.AgentId, 
 x.Latitude, x.Longitude,
 DistanceToCustomer = ws.GetDistanceMeters(x.Latitude, x.Longitude, customerLatitude, customerLongitude) 
 )
 .OrderBy(x => x.DistanceToCustomer);
foreach (var agent in nearestAgents.Take(5))

 Console.WriteLine($"agent_id: agent.AgentId distance: agent.DistanceToCustomer m");

The .Take(5) ensures that only 5 agent will be printed out, or less.

I'd say you have the following options:

1. 
   Keep that new class if you think it's relevant in your business. Here, it should be important to know if the logic of calculate the distance should be on that web service (I assume that 'ws' variable means that) or within your model (i.e., the 'Agent' class).


2. 
   Use an anonymous class if you think you won't pass that info to another method.


3. 
   Use a dictionary if you think it's not relevant in your business but will pass that info to another method.

Personally, I'd go for the first one since it's the most natural to me. By the way, I'd use LINQ's OrderBy instead of implementing the IComparable interface; again, for expressiveness.

I think, I would hold on to your decorator pattern because it is more reusable than the selection with anonymous objects.

 public class DistancedAgent : Agent
 
 public DistancedAgent(Agent source, double distance)
 
 AgentId = source.AgentId;
 Latitude = source.Latitude;
 Longitude = source.Longitude;
 Distance = distance;
 

 public double Distance get; 

 public override string ToString()
 
 return $"Latitude, Longitude => Distance";
 
 

You could extent Agent with some converter methods:

 public static class Extensions
 
 public static DistancedAgent WithDistance(this Agent agent, Func<Agent, double> getDistance)
 
 return new DistancedAgent(agent, getDistance(agent));
 

 public static DistancedAgent WithDistance(this Agent agent, IDistanceProvider distanceProvider)
 
 return new DistancedAgent(agent, distanceProvider.GetDistance(agent));
 

 public static IEnumerable<DistancedAgent> WithDistance(this IEnumerable<Agent> agents, Func<Agent, double> getDistance)
 
 return agents?.Where(a => a != null).Select(a => a.WithDistance(getDistance)) ?? new DistancedAgent[0];
 

 public static IEnumerable<DistancedAgent> WithDistance(this IEnumerable<Agent> agents, IDistanceProvider distanceProvider)
 
 return agents?.Where(a => a != null).Select(a => a.WithDistance(distanceProvider)) ?? new DistancedAgent[0];
 
 

where IDistanceProvider is

 public interface IDistanceProvider
 
 double GetDistance(Agent agent);
 

In the concrete use case it ends up with code like this:

 var agents = db.GetAgents();

 Func<Agent, double> getDistance = a => ws.GetDistanceMeters(a.Latitude, a.Longitude, customerLatitude, customerLongitude);

 foreach (DistancedAgent dAgent in agents.WithDistance(getDistance).OrderBy(da => da.Distance).Take(5))
 
 Console.WriteLine(dAgent);
 

which is easy to understand and maintain and you have a setup that can be used wherever needed.