Merge pull request #610 from arne182/patch-2

Fix logic for grouping consecutive points in CountriesAndCities
This commit is contained in:
Evgenii Burmakin 2025-05-29 12:42:01 +02:00 committed by GitHub
commit 05018b6e6c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 75 additions and 59 deletions

View file

@ -12,43 +12,64 @@ class CountriesAndCities
points
.reject { |point| point.country.nil? || point.city.nil? }
.group_by(&:country)
.transform_values { |country_points| process_country_points(country_points) }
.map { |country, cities| CountryData.new(country: country, cities: cities) }
.map do |country, country_points|
cities = process_country_points(country_points)
CountryData.new(country: country, cities: cities) if cities.any?
end.compact
end
private
attr_reader :points
# Step 1: Process points to group by consecutive cities and time
def group_points_with_consecutive_cities(country_points)
sorted_points = country_points.sort_by(&:timestamp)
sessions = []
current_session = []
sorted_points.each_with_index do |point, index|
if current_session.empty?
current_session << point
next
end
prev_point = sorted_points[index - 1]
# Split session if city changes or time gap exceeds the threshold
if point.city != prev_point.city
sessions << current_session
current_session = []
end
current_session << point
end
sessions << current_session unless current_session.empty?
sessions
end
# Step 2: Filter sessions that don't meet the minimum minutes per city
def filter_sessions(sessions)
sessions.map do |session|
end_time = session.last.timestamp
duration = (end_time - session.first.timestamp) / 60 # Convert seconds to minutes
if duration >= MIN_MINUTES_SPENT_IN_CITY
CityData.new(
city: session.first.city,
points: session.size,
timestamp: end_time,
stayed_for: duration
)
end
end.compact
end
# Process points for each country
def process_country_points(country_points)
country_points
.group_by(&:city)
.transform_values { |city_points| create_city_data_if_valid(city_points) }
.values
.compact
end
def create_city_data_if_valid(city_points)
timestamps = city_points.pluck(:timestamp)
duration = calculate_duration_in_minutes(timestamps)
city = city_points.first.city
points_count = city_points.size
build_city_data(city, points_count, timestamps, duration)
end
def build_city_data(city, points_count, timestamps, duration)
return nil if duration < ::MIN_MINUTES_SPENT_IN_CITY
CityData.new(
city: city,
points: points_count,
timestamp: timestamps.max,
stayed_for: duration
)
end
def calculate_duration_in_minutes(timestamps)
((timestamps.max - timestamps.min).to_i / 60)
sessions = group_points_with_consecutive_cities(country_points)
filter_sessions(sessions)
end
end

View file

@ -6,24 +6,29 @@ RSpec.describe CountriesAndCities do
describe '#call' do
subject(:countries_and_cities) { described_class.new(points).call }
# we have 5 points in the same city and country within 1 hour,
# 5 points in the differnt city within 10 minutes
# and we expect to get one country with one city which has 5 points
# we have 15 points in the same city and different country within 2 hour,
# 4 points in the differnt city within 10 minutes splitting the country
# and we expect to get one country with one city which has 8 points
let(:timestamp) { DateTime.new(2021, 1, 1, 0, 0, 0) }
let(:points) do
[
create(:point, city: 'Berlin', country: 'Germany', timestamp:),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 10.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 20.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 30.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 40.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 50.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 60.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 70.minutes),
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 80.minutes),
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes)
create(:point, city: 'Kerpen', country: 'Belgium', timestamp:),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 10.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 20.minutes),
create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 30.minutes),
create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 40.minutes),
create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 50.minutes),
create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 60.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 70.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 80.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 90.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 100.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 110.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 120.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 130.minutes),
create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 140.minutes)
]
end
@ -37,16 +42,12 @@ RSpec.describe CountriesAndCities do
expect(countries_and_cities).to eq(
[
CountriesAndCities::CountryData.new(
country: 'Germany',
country: 'Belgium',
cities: [
CountriesAndCities::CityData.new(
city: 'Berlin', points: 8, timestamp: 1_609_463_400, stayed_for: 70
city: 'Kerpen', points: 8, timestamp: 1_609_467_600, stayed_for: 70
)
]
),
CountriesAndCities::CountryData.new(
country: 'Belgium',
cities: []
)
]
)
@ -60,21 +61,15 @@ RSpec.describe CountriesAndCities do
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 10.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 20.minutes),
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 80.minutes),
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes)
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes),
create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 100.minutes),
create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 110.minutes)
]
end
it 'returns countries and cities' do
expect(countries_and_cities).to eq(
[
CountriesAndCities::CountryData.new(
country: 'Germany',
cities: []
),
CountriesAndCities::CountryData.new(
country: 'Belgium',
cities: []
)
]
)
end