【Ruby】オブジェクト指向で、もうちょいセンスのあるコードに書き換えたい

Question

テーマは、「フェラーリは車高を40cmリフトアップしたら速度が20%遅くなる」ということをコードにしたいです。
前提条件として「リフトダウンはリフトアップしてからじゃないとできない」ということを ferrari.rb に定義しました。（リフトアップ/ダウンはフェラーリだけしかできないので）
定義の仕方がいまいち弱いというか、これだと定義しなくても putsの中の計算式でなんとかなるくね？
と思ってしまい、なんか違うなぁと。

直したい箇所は ferrari.rb のdef lift_down の「時速」と「車高」を def lift_up で計算された数値から計算されるようなコードにしたいのですがどうすればいいでしょうか？

lift_upで計算した数値を基準に元に戻そうと#{car.lift_up.speed /0.8}km とかにしたらエラーが出るし、どうすればいいか悩んでいまして、アドバイスお願いいたします。

# main.rb
require_relative 'car'
require_relative 'ferrari'
require_relative 'honda'
require_relative 'nissan'

honda = Honda.new
honda.output_information(honda)

nissan = Nissan.new
nissan.output_information(nissan)

ferrari = Ferrari.new
ferrari.output_information(ferrari)

ferrari.lift_up(ferrari)
ferrari.lift_down(ferrari)

#car.rb (parent class)
class Car
  attr_accessor :type, :capasity, :price, :speed, :height

  def equipment
    %w[アクセル ブレーキ]
  end

  def output_information(car)
    puts '---------------------------------'
    puts "車種: #{car.type}"
    puts "定員: #{car.capasity}"
    puts "価格: #{car.price}円"
    puts "時速: #{car.speed}km"
    puts "車高: #{car.height}cm"
    puts "装備: #{car.equipment.join(', ')}"
  end
end

# ferrari.rb
class Ferrari < Car
  def initialize 
    @type = 'フェラーリ'
    @capasity = 2
    @price = 2_000_000_0
    @speed = 100
    @height = 100
  end
  
  def lift_up(car)
    @is_lift = true
    puts '----フェラーリ（リフトアップ）----'
    puts "車種: #{car.type}"
    puts "定員: #{car.capasity}"
    puts "価格: #{car.price}円"
    puts "時速: #{car.speed* 0.8}km"
    puts "車高: #{car.height+ 40}cm"
    puts "装備: #{car.equipment.join(', ')}"
  end
  
  def  lift_down(car)
    return unless @is_lift
    @is_lift = false
    puts '----フェラーリ（リフトダウン）----'
    puts "車種: #{car.type}"
    puts "定員: #{car.capasity}"
    puts "価格: #{car.price}円"
    puts "時速: #{(car.speed* 0.8)/ 0.8}km" #ここをリフトアップした数値を基準に元の数値に戻したい
    puts "車高: #{(car.height+ 40) - 40}cm" #ここをリフトアップした数値を基準に元の数値に戻したい
    puts "装備: #{car.equipment.join(', ')}"
  end
end

# honda.rb
class Honda < Car
  def initialize
    @type = 'ホンダ'
    @capasity = 8
    @price = 100_000_0
    @speed = 10
    @height = 100
  end
end

# nissan.rb
class Nissan < Car
  def initialize
    @type = 'ニッサン'
    @capasity = 5
    @price = 50_000_0
    @speed = 10
    @height = 100
  end
end

下記は出力した時のスクショです。

Answer 1

#car.rb (parent class)
class Car
  # capasity -> capacity
  attr_accessor :type, :capacity, :price, :speed, :height

  def equipment
    %w[アクセル ブレーキ]
  end

  def output_information
    puts '---------------------------------'
    puts "車種: #{type}"
    puts "定員: #{capacity}"
    puts "価格: #{price}円"
    puts "時速: #{speed}km"
    puts "車高: #{height}cm"
    puts "装備: #{equipment.join(', ')}"
  end
  
  def initialize(type: '(車種)', capacity: 1, price: 0, speed: 10, height: 100)
    @type = type
    @capacity = capacity
    @price = price
    @speed = speed
    @height = height
  end
end


# ferrari.rb
class Ferrari < Car
  def initialize
    # 親(Car)のinitializeを呼び出す
    # 親クラスのインスタンス変数を直接操作するのは、親クラスの仕様変更に弱い
    super(
      type: 'フェラーリ', capacity: 2,
      price: 2_000_000_0, speed: 100
    )
    output_information
  end

  def lift_up
    @is_lift = true
    # selfの状態を変更し
    self.type = 'フェラーリ（リフトアップ）'
    self.speed *= 0.8
    self.height += 40
    # 出力は親クラスのメソッドに任せる
    output_information
  end

  def  lift_down
    # 何もせずreturnするだけよりは、呼び出し元に対して例外を返した方が親切
    raise "リフトアップしていない車はリフトダウンできません" unless @is_lift
    @is_lift = false
    self.type = 'フェラーリ（リフトダウン）'
    self.speed /= 0.8
    self.height -= 40
    output_information
  end
end

ferrari = Ferrari.new
ferrari.output_information
ferrari.lift_up
ferrari.lift_down

全体的に、carを引数に取る必要はなく(必要ならば)selfを使います。

Comments

[kazuki0714]

ありがとうございます！
めっちゃ分かりやすいです！

早速参考にさせていただきます！

[asm]

あとは、lift_up lift_downが2つの事（selfの状態変化・状態表示)を行っている事が気になっています。
「単一責任の原則」のような、ちょっと違うような微妙な感じなのですが

果たして、lift_upというメソッド名から表示まで行うことを推測できるかというと微妙ですし
変更・表示が１つのメソッドというのは違和感あります。
(かといって、分けるとmain側がごちゃっとするので今回は見逃しておきます)

[kazuki0714]

output_informationを違うメソッドで定義するほうがいいということでしょうか？
（例えば `lift_up_output` のようなメソッドで）

[asm]

lift_upは変更のみで、表示したいのならば各々output_informationを実行する感じですね

ただ、記述量が単純に倍になってめんどくさいですし
変更を実行し、結果を表示するための整備工場的なクラスを間に挟んで
利用者はそこに車を持ち込んで注文する・・・てのが個人的にはスッキリするかな　と思います。

自分だけしか開発に関わってなく利用者も自分だけという小規模ならば、あえて無視する程度の問題です

[kazuki0714]

なるほどですね、ありがとうございます！
勉強用に自分1人で書いてたのでそこまで意識が回ってませんでした(>_<)