Can I ask you for a code review?

Question

Ruby on Rails

Good day.
I am developing an application on RoR, and I do it alone. I never found a mentor.
I would like to ask you to help in terms of code criticism, because no one to ask.
To better understand what the code does in general, you can look at this page:
https://skyderby.ru/ru/tracks/2795/replay
In general, there is a GPS flight track, you need to evaluate it in terms of the best range for 1000 meters of altitude, the best speed for 1000 meters of altitude, the best time for 1000 meters of altitude. At the same time, if the flight in the track is less than 1000 meters or it is a flight from rocks (base jumping), then you need to calculate the results for the entire range.
For these purposes, I developed a service object:

class TrackResultsService
  attr_reader :track

  STEP_SIZE = 50
  SKYDIVE_RANGE = 1000

  def initialize(trk)
    @track = trk
  end

  def execute
    track.destroy_results  
    
    results = ranges_to_score.map do |range|
      { range: range,
        track_segment: JumpRangeFinder.new(track_points)
                                      .execute(from_altitude: range[:start_altitude],
                                               to_altitude:   range[:end_altitude])
      }
    end

    [:speed, :distance, :time].each do |task|
      best_task_result = results.max_by { |x| x[:track_segment][task] }
      track.track_result << TrackResult.new(discipline: task,
                                            range_from: range[:start_altitude],
                                            range_to: range[:end_altitude],
                                            result: best_task_result)
    end
  end

  def track_points
    @track_points ||=
      track.points.trimmed.pluck_to_hash(
        'to_timestamp(gps_time_in_seconds) AT TIME ZONE \'UTC\' as gps_time',
        "#{@track.point_altitude_field} AS altitude",
        :latitude,
        :longitude)
  end

  def ranges_to_score
    altitude_bounds = track.altitude_bounds
    
    # round to STEP_SIZE. Max to lower, min to upper value
    max_altitude = altitude_bounds[:max_altitude] - altitude_bounds[:max_altitude] % STEP_SIZE
    min_altitude = altitude_bounds[:min_altitude] + STEP_SIZE - altitude_bounds[:min_altitude] % STEP_SIZE
    elevation = max_altitude - min_altitude

    return [{
      start_altitude: altitude_bounds[:max_altitude],
      end_altitude: altitude_bounds[:min_altitude]
    }] if track.base? || elevation <= SKYDIVE_RANGE

    ranges = []
    steps = ((elevation - SKYDIVE_RANGE) / STEP_SIZE).floor
    steps.times do |step|
      altitude_change = STEP_SIZE * step
      ranges << { 
        start_altitude: altitude_bounds[:max_altitude] - altitude_change,
        end_altitude:   altitude_bounds[:max_altitude] - SKYDIVE_RANGE - altitude_change
      }
    end

    ranges
  end
end

Please give some feedback.

Answer 1

Hello, in principle the code is not bad, I can only give a few general tips
Short variable names are a bad habit, even if they are used immediately
Clearly separate public and private methods
In the execute method, results is not the best variable name, because these are only intermediate results, the result itself is built in the last cycle
Regardless of whether you write the code yourself or in a team, you should always try to write the code as if it were written in a team. For example, for the function track_points does not place a comment on the example of the resulting hash that it will return. With keys and values. Whoever reads this code will save a lot of time
the ranges_to_score method, although too long according to ruby ​​canons, looks pretty coherent, the only remark is if track.base? || elevation <= SKYDIVE_RANGE in post condition - sugar abuse. This if is almost invisible there.
To this method, I would also describe the format of the returned array
with a comment
I leave to the code of my Padawans.