friend locator app

Question

I was rejected after a tech screening for the following code. I mainly struggled to make the code 'production ready'. I was running out of time so I did not create a class for testing. How could I have improved this code?

/*
Problem
Imagine you are building a friend locator app.
Given your current position identified on a bidimensional plane as (0,0) and a list of friend locations,
each identified by a (x,y) pair, return the k closest friends to your current location.

For simplicity, you can compute the distance between you and your friends using the Euclidean distance formula ( d = sqrt(x^2 +y^2) ).

Given a list of friend locations [[1,3], [-2,2]], K = 1
Output: [[-2,2]]
*/

import java.io.*;
import java.util.*;

/*
 * To execute Java, please define "static void main" on a class
 * named Solution.
 *
 * If you need more classes, simply define them inline.
 */

class Solution {
  
  static class Pair{
    int x, y;
    
    Pair(int x, int y){
     this.x = x;
      this.y = y;
    }
      
  }
  
  /*
  d -> cords
  time - O(n)
  space - O(n)
  */
  public static List<Pair> closestFriends(List<Pair> list, int k){
    
  
    SortedMap<Double, Pair> map = new TreeMap<>();
    List<Pair> results = new ArrayList<Pair>();
    int i =0;
    
    for(Pair pair : list){
      double d = distance(pair);
      map.put(d, pair);
    }
    
    for(Map.Entry<Double, Pair> entry : map.entrySet()){
      if(i < k){
        results.add(entry.getValue()); 
        i++;
      }
    }
    
    return results;
    
  }
  
  public static double distance(Pair p){
    return Math.sqrt(p.x*p.x + p.y*p.y);
  }
  
  
  public static void main(String[] args) {
    
    ArrayList<Pair> cords = new ArrayList<>();
    cords.add(new Pair(1,3));
    cords.add(new Pair(-2,2));
    cords.add(new Pair(0,10));
    cords.add(new Pair(-2,2));
    cords.add(new Pair(1,3));
    cords.add(new Pair(-2,2));
    
    int k =1;
    /*
    empty list of cords
    negative k
    list list of cords
    test cases for distance - correct calculation      
    */
    
    List<Pair> res = closestFriends(cords, k);
    
    for(int i =0; i < res.size(); i++){
      System.out.println(res.get(i).x + "," +res.get(i).y); 
    }
    
    
  }
}

Answer 1

major issue: misunderstanding of exercise

you should find the amount k of friends - not the friends within an distanz k

major issue: bug

for(Pair pair : list){
    double d = distance(pair);
    map.put(d, pair);
}

if two "pair"s have the same distance you'll lose one result

major issue: naming

sounds obvious but you should create a Position not a Pair (also named in your exercise as Location - but never a Pair)

minor issue: Map instead of a List (mostly assumption on this)

your exercise wants you to return a the first k items of a sorted list. I had expected you to use a List and create a Comparator, sort the list and then return the k first entries. you use a map to sort the element. and it's a buggy (see above) algorithm to sort.

minor issue: Java coding conventions, not considering encapsulation

as example I say that int x, y; should be:

int x;
int y;

and formatting, use the spaces properly

minor issue: naming

• when they give you the amount k you could pick up and rename that variable into a proper name
• when they give you a list of friends that you should filter rename that list properly

public static List<Pair> closestFriends(List<Pair> friends, int amount)...

another one here: I'm missing a proper verb in your method name: getClosestFriends() or maybe findClosestLocations()