Popescu Alexandru Interview by popescualexandru9 · Pull Request #4 · Agilefreaks/ruby-interview-starting-point

popescualexandru9 · 2025-01-28T16:14:51Z

Instructions

Refer to README

Provided Coding challange description

Overview

You have been hired by a company that builds a app for coffee addicts. You are
responsible for taking the user’s location and returning a list of the three closest coffee shops.

Starting point

Fork or clone our ruby starter project: https://github.com/Agilefreaks/ruby-interview-starting-point

Input

The coffee shop list is a comma separated file with rows of the following form:
Name,Y Coordinate,X Coordinate

The quality of data in this list of coffee shops may vary. Malformed entries should cause the
program to exit appropriately.

Notice that the data file will be read from an network location
(ex: https://raw.githubusercontent.com/Agilefreaks/test_oop/master/coffee_shops.csv)

Output

Write a REST API in any framework you wish (excluding Rails) that offers the posibility to take the user's coordinates and
return a list of the three closest coffee shops (including distance from the user) in
order of closest to farthest. These distances should be rounded to four decimal places.

Assume all coordinates lie on a plane.

Example

Using the coffee_shops.csv

Input

47.6 -122.4

Expected output

Starbucks Seattle2,0.0645
Starbucks Seattle,0.0861
Starbucks SF,10.0793

pirvudoru

Thanks for the solution. Please take the time to answer the questions. No need to do code changes.

pirvudoru · 2025-01-29T09:35:20Z

+    shops_with_distances = coffee_shop_finder_service.closest_shops(lat, lon)
+    format_response(shops_with_distances, lat, lon)
+  rescue StandardError => e
+    handle_error(e)


Why does success returns a plain/text and failure returns a json?

Hey! I'll answer here for 2 other comments as well to keep the conversation in one place since they're all related.
It is actually a mistake. I intended to use application/json everywhere, but while debugging the specs for CoffeeShopController I tried multiple approaches and forgot to roll-back this change 😅

Related comments: Why set it here as json, and override it on line 20? , Why did you choose this content type?

pirvudoru · 2025-01-29T09:35:48Z

+
+  private
+
+  # Format shops into "Name --> distance <-- (user-lat, user_lon)" strings


Why this format? What's the intended audience?

The format from that comment is outdated as well, it actually returns:

Coffee shops nearest (user_lat, user_lon) by distance: Distance <--> Name

I meant to match the __Expected output__ in a more readable format

pirvudoru · 2025-01-29T09:41:21Z

+  end
+
+  before do
+    content_type :json


Why set it here as json, and override it on line 20?

Refer to: Why does success returns a plain/text and failure returns a json?

pirvudoru · 2025-01-29T09:42:01Z

+  end
+
+  get '/closest_shops' do
+    content_type 'text/plain'


Why did you choose this content type?

Refer to: Why does success returns a plain/text and failure returns a json?

pirvudoru · 2025-01-29T09:44:03Z

+                  when /Invalid CSV/ then 400
+                  when /Failed to fetch CSV/ then 502
+                  else 500
+                  end


Any other alternative considered? Can you share the pros and cons of different approaches?

Yes so I choose this approach because it was simple and fast to implement for this particular coding exercise.

It's not a scalable solution though, other better approaches I think would be:

custom exception classes - similar to what I've done in CoffeeShop model class

I think this would've been the best approach since it provides type checking, it's easy to add metadata to errors and you can create some sort of logical hierarchy for errors which is also more readable. Also framework agnostic

The drawback on speed ( initial class setup ) is minimal

using a framework-specific handling

Biggest con is that I'm not familiar with Sinatra

drqCode · 2025-01-29T09:58:47Z

+  def handle_error(error)
+    status_code = case error.message
+                  when /Invalid CSV/ then 400
+                  when /Failed to fetch CSV/ then 502


502 is not correct here according to industry standards: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/502

"The HTTP 502 Bad Gateway server error response status code indicates that a server was acting as a gateway or proxy and that it received an invalid response from the upstream server. This response is similar to a 500 Internal Server Error response in the sense that it is a generic "catch-call" for server errors."

Our server does not act as a gateway or proxy, although it requests another server. In our case, the remote CSV acts as a database

I agree with you. I treated the remote CSV as another server, so 502 made sense in my head, but if it mimics our said DB, code 500 would make more sense 🧠

drqCode · 2025-01-29T10:02:55Z

+  # Handle errors with appropriate HTTP status codes
+  def handle_error(error)
+    status_code = case error.message
+                  when /Invalid CSV/ then 400


400 is not correct here according to industry standards: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400.

"The HTTP 400 Bad Request client error response status code indicates that the server would not process the request due to something the server considered to be a client error. The reason for a 400 response is typically due to malformed request syntax, invalid request message framing, or deceptive request routing.

Clients that receive a 400 response should expect that repeating the request without modification will fail with the same error."

It's not requester fault that the server's data is invalid.

The requester may not modify the request and stop receiving 400 at later times

Yea correct this is a mistake. The fact that our DB data is invalid is not on the client side.
I should've use code 500 or 422 Unprocessable Content - "status code indicates that the server understood the content type of the request content, and the syntax of the request content was correct, but it was unable to process the contained instructions."

drqCode · 2025-01-29T10:06:13Z

+    Float(str)
+  end
+
+  # Handle errors with appropriate HTTP status codes


This comment doesn't tell more than what the code tells. Comments have a specific purpose throughout the code. You can check this paper for when comments might be a good idea: https://stackoverflow.blog/2021/12/23/best-practices-for-writing-code-comments/

This should be deleted as well: # Validate CSV row structure 🗑️

Alexandru Popescu added 3 commits January 28, 2025 18:13

Add initial configuration and CoffeeShop model

9231f80

Add Coffee Shops Finder service

2fb70a4

Add controller and test endpoint

500d69d

popescualexandru9 changed the title ~~Develop~~ Popescu Alexandru Interview Jan 28, 2025

Update README.md

6d5e674

popescualexandru9 force-pushed the develop branch from 7657760 to 6d5e674 Compare January 28, 2025 16:51

Update README.md

f64934a

pirvudoru reviewed Jan 29, 2025

View reviewed changes

drqCode reviewed Jan 29, 2025

View reviewed changes


		private

		# Format shops into "Name --> distance <-- (user-lat, user_lon)" strings

Conversation

popescualexandru9 commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instructions

Overview

Starting point

Input

Output

Example

Uh oh!

pirvudoru left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popescualexandru9 Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

popescualexandru9 commented Jan 28, 2025 •

edited

Loading

popescualexandru9 Jan 29, 2025 •

edited

Loading