I have a situation where I need to predict the input values for a form used to create a new model. The model is a "class report" that a teacher submits for a student after their class finishes and the values of the report I need to predict include the time the class started and finished. These are to be predicted based on previously submitted reports and other model data. When a teacher opens the form to submit a class report, they should see the "class start time" and other values filled in for them automatically with the predictions made.
Currently, this is being handled in the model as one single method and I am looking for a way to refactor it into its own class:
class ClassReport
...
# Get predicted values for report form for a given student, teacher and weekday.
# e.g. if a student and teacher had a class on the same day as the current day
# last week, use the same start and end times as the defaults.
#
# Values predicted are:
# - class start time
# - class finish time
# - date of next class
# - start time of next class
def self.predict_fields(student, teacher, day)
last_report = ClassReport.most_recent(student, teacher)
last_report_for_today = ClassReport.most_recent_for_wday(student, teacher, day.wday)
if Matching.has_class_today?(student, teacher)
# Get schedule from calendar
class_start, class_finish = Matching.get_class_time(student, teacher)
elsif last_report.present?
if last_report_for_today.present?
# Use start time from last report for this weekday
class_start = last_report_for_today.class_start
else
# Use start time from last report
class_start = last_report.class_start
end
if last_report_for_today.present?
# Use duration from last report for current weekday
class_finish = class_start + last_report_for_today.class_minutes.minutes
else
# Default duration of 1 hour
class_finish = class_start + 1.hour
end
else
# Set time to 1 hour before now
# Default duration of 1 hour
class_start = Time.now-1.hour
class_finish = Time.now
end
# Get next class day and try and get last report for that weekday
next_class_wday = Matching.next_class_wday(student, teacher)
if next_class_wday.present?
next_report = ClassReport.most_recent_for_wday(student, teacher, next_class_wday)
end
if next_report.present?
# Use next class day and time from previous report for this weekday
next_report_day = next_report.class_day
next_report_time = next_report.class_start
else
if next_class_wday.present?
# Use next matching day as date
date_diff = (day.wday - next_class_wday).abs
next_report_day = day + date_diff.days
else
# One week from now
next_report_day = day + 1.week
end
if last_report.present?
# Set time to same as previous report
next_report_time = last_report.class_start
else
# Set time to now-1h
next_report_time = Time.now-1.hour
end
end
[class_start, class_finish, next_report_day, next_report_time]
end
You can see that this uses multiple models and is doing far too much for a single method. Ideally I would want this as a class that takes student
, teacher
and day
in the constructor, does some initialisation, and has methods for ReportPrediction#class_start
, ReportPrediction#class_finish
, etc.
I'm not sure if this is the best approach though, or where such a class should be placed. I had a read through these refactoring patterns but couldn't see any that I thought fitted this situation. The closest seemed to be a "policy object" but that appears to be more commonly used for returning boolean values based on model data. Service objects also seem slightly different as the way the author describes them implies they are for write operations.
How would you refactor this in a Rails-like manner? Or in more general terms, where would you have logic that looks at the attributes of multiple models and gives you back some values based on those?
1 Answer 1
For any complex piece of logic that involves multiple models, I'd rely on a service. Here's how I'd refactor your method into services:
#result= PredictFieldService.build.call(student, teacher, day)
class PredictFieldService
def self.build
new(ClassStartFinishService.build, .....)
end
def initialize(class_start_finish_service, .......)
@class_start_finish_service = class_start_finish_service
........
end
def call(student, teacher, day)
last_report = get_last_report
last_report_for_today = get_last_report_for_today
class_start, class_finish = @class_start_finish_service.call(student, teacher, last_report, last_report_for_today)
next_report_day = anyservice.call
next_report_time = whateverservice.call
[class_start, class_finish, next_report_day, next_report_time]
end
private
def get_last_report
ClassReport.most_recent(student, teacher)
end
def get_last_report_for_today
ClassReport.most_recent_for_wday(student, teacher, day.wday)
end
end
class PredictFieldService::ClassStartFinishService
def self.build
new
end
def call(student, teacher, last_report, last_report_for_today)
if Matching.has_class_today?(student, teacher)
class_start, class_finish = get_matching_class
else
class_start, class_finish = get_start_finish(last_report, last_report_for_today)
end
class_start, class_finish
end
private
def get_matching_class
Matching.get_class_time(student, teacher)
end
def get_start_finish(last_report, last_report_for_today)
if last_report.present?
if last_report_for_today.present?
class_start = last_report_for_today.class_start
class_finish = class_start + last_report_for_today.class_minutes.minutes
else
class_start = last_report.class_start
class_finish = class_start + 1.hour
end
else
class_start = Time.now-1.hour
class_finish = Time.now
end
[class_start, class_finish]
end
end
This way you can test each piece of logic encapsulated in a service in isolation, and expand the functionality through dependency injection.