I have an app that aims to helps people to meet up. I have a database table that stores time windows of availability for a particular user:
AvailabilityWindow (user_id, start_time, end_time, can_travel, travel_radius, target_user, will_travel, state)
(state can be 'active' or 'removed')
User availability can be open to all other users, or targeting a specific user (by using the target_user_id
field in AvailabilityWindow
). To make creating untargeted/targeted availability easier, I have two classes:
class UserOpenAvailability
class << self
def for(user)
new(user, user.availability_windows.where(target_user_id: nil))
end
end
def initialize(user, availability: [])
@user = user
@availabilities = availability
end
def add(start_time, end_time, can_travel: false, travel_radius: 0)
@availabilities << AvailabilityWindow.new(user: user, start_time: start_time, end_time: end_time, can_travel: can_travel, travel_radus: travel_radius)
end
def existing_availability
self.class.for(@user)
end
def save
# complicated save logic
end
end
class TargetedUserAvailability
class << self
def for(user, target_user)
new(user, target_user, user.availability_windows.where(target_user: target_user))
end
end
def initialize(user, target_user, availability: [])
@user = user
@target_user = target_user
@availabilities = availability
end
def add(start_time, end_time, can_travel, travel_radius, will_travel: true)
@availabilities << AvailabilityWindow.new(user: user, target_user: @target_user, start_time: start_time, end_time: end_time, can_travel: can_travel, travel_radus: travel_radius, will_travel: will_travel)
end
def existing_availability
self.class.for(@user, @target_user)
end
def save
#...
end
end
These can be used as follows to either add to existing availability:
availability = UserOpenAvailability.for(user)
availability.add(...)
availability.save
or save a fresh set of availability:
availability = UserOpenAvailability.new(user, [])
availability.add(...)
availability.save
The problem I'm having is that the logic in the save
method is extensive, and very similar in both classes.
When a user updates their availability, I need to add/update records in the AvailabilityWindow
table.
- any new availability needs to be added as a new record
- any previous availability that has been deselected needs to be 'removed' by changing the
state
toremoved
- any changes to existing availability (e.g. end_time brought forward) requires the existing availability to be removed and a new, reduced time slot added.
- any unchanged availability needs to be untouched
Implementing the above logic is fairly complicated. It needs to be wrapped in a transaction, it needs to compare existing availability with new availability to work out what needs to be saved/removed, it needs to save all the updates and report if any changes ended up being made.
At the moment it looks kind of like this:
def save
@user.with_lock do
ActiveRecord::Base.transaction do
added_availabilities.each do |availability|
raise ActiveRecord::Rollback unless availability.save
end
removed_availabilities.each(&:remove!)
@availability_changed = (added_avaialbilities + removed_availabilities).any?
return true
end
false
end
end
def availability_changed?
!!@availability_changed
end
def added_availabilities
@availabilities.select do |new_availability|
existing_availabilities.none? do |existing|
existing.same_availability?(new_availability)
end
end
end
def removed_availabilities
existing_availabilities.select do |existing|
@availabilities.none? do |new_availability|
new_availability.same_availability?(existing)
end
end
end
This code is currently sitting in both classes and I'm having trouble refactoring it out. I don't think creating a base class that both my classes extend from really works because the #add
methods have different arguments.
I could move the logic into a concern/module, but then there's going to be bidirectional dependencies as the concern expects there to be both a @availabilites
array and existing_availabilities
method. That seems like I'm just hiding the problem.
I can only think of adding a class such as AvailabilityWindowUpdater
that takes a set of existing availability windows and a desired set of availability windows, and performs all the above save logic. But having a class called ..Updater
is usually not a good sign.
Can anyone help with some suggestions?
Aucun commentaire:
Enregistrer un commentaire