Xjyuk

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.