1
\$\begingroup\$

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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 8, 2016 at 20:19
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

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.

answered Jun 20, 2016 at 19:34
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.