Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SwordWorld2.0] レーティング表部分の解析結果のリファクタリング #516

Open
ochaochaocha3 opened this issue Nov 24, 2021 · 0 comments
Labels
refactoring 内部構造の改良

Comments

@ochaochaocha3
Copy link
Member

ochaochaocha3 commented Nov 24, 2021

ソード・ワールド2.0、2.5のレーティング表部分の解析結果(rating_parsed.rb)の冗長な部分を簡潔にする。

現状

rating_parsed.rbでは、各インスタンス変数を nil で初期化しており、またそれらを正規化するメソッド(nil ならば 0 等の既定値に変換したり、範囲内に収めたりするメソッド)を用意している。

def initialize
@critical = nil
@kept_modify = nil
@first_to = nil
@first_modify = nil
@greatest_fortune = false
@rateup = nil
end

正規化メソッドの例:

# @return [Integer]
def first_modify
return @first_modify || 0
end

以下に示すように、これらはうまく使われていない。

rating_parsed.rbの #to_s には、必要なパーツを文字列に追加する処理が書かれているが、必要性の判断は nil との比較ではなく、0 等の既定値との比較で行われている。また、条件判定と式展開で正規化メソッドが2回呼び出されているため、インスタンス変数と既定値の比較が最大で3回行われている。

def to_s()
output = "KeyNo.#{@rate}"
output += "c[#{critical}]" if critical < 13
output += "m[#{Format.modifier(first_modify)}]" if first_modify != 0
output += "m[#{first_to}]" if first_to != 0
output += "r[#{rateup}]" if rateup != 0
output += "gf" if @greatest_fortune
output += "a[#{Format.modifier(kept_modify)}]" if kept_modify != 0
if @modifier != 0
output += Format.modifier(@modifier)
end
return output
end

レーティング表の処理はSwordWorld.rbに書かれているが、ここでも必要性の判断は nil との比較ではなく、0 等の既定値との比較で行われている。

例1:

first_to = command.first_to
first_modify = command.first_modify
loop do
dice_raw, diceText = rollDice(command)
dice = dice_raw
if first_to != 0
dice = dice_raw = first_to
first_to = 0
elsif first_modify != 0
dice += first_modify
first_modify = 0
end

例2:

# rate回数が1回で、修正値がない時には途中式と最終結果が一致するので、途中式を省略する
if rateResults.size > 1 || command.modifier != 0
text = rateResults.join(',') + Format.modifier(command.modifier)
if command.half
text = "(#{text})/2"
if command.modifier_after_half != 0
text += Format.modifier(command.modifier_after_half)
end
end
sequence.push(text)
elsif command.half
text = "#{rateResults.first}/2"
if command.modifier_after_half != 0
text += Format.modifier(command.modifier_after_half)
end
sequence.push(text)
end

以上の例のように、各インスタンス変数の nil は使われておらず、0 等の既定値を直接設定すればよくなっている。

おそらく、構文解析器の #parsed にある、値をまとめて設定する処理との整合性をとるために、このような形になったのではないだろうか。

def parsed(rate, modifier, option)
RatingParsed.new.tap do |p|
p.rate = rate
p.critical = option[:critical]&.eval(@round_type)
p.kept_modify = option[:kept_modify]&.eval(@round_type)
p.first_to = option[:first_to]
p.first_modify = option[:first_modify]
p.rateup = option[:rateup]&.eval(@round_type)
p.greatest_fortune = option.fetch(:greatest_fortune, false)
p.semi_fixed_val = option[:semi_fixed_val]
p.tmp_fixed_val = option[:tmp_fixed_val]
p.modifier = modifier.eval(@round_type)
p.modifier_after_half = option[:modifier_after_half]&.eval(@round_type)
end
end

変更案

RatingParsedの各インスタンス変数を原則として 0 等の既定値で初期化するようにする。そうすれば、正規化メソッドは不要になり、直接インスタンス変数を参照すればよくなる。nil での初期化のままにし、SwordWorld.rb側も含めて処理を書き換えることも一案だが、範囲内に収める等の処理もあるため、数値で表現しておく方が問題が生じにくいと思われる。

class RatingParsed
  def initialize
    @critical = nil
    @kept_modify = 0
    @first_to = 0
    @first_modify = 0
    @greatest_fortune = false
    @rateup = 0
  end

  # @return [Boolean]
  def half
    @modifier_after_half != 0
  end

  # @return [String]
  def to_s()
    sequence = ["KeyNo.#{@rate}"]

    sequence << "c[#{@critical}]" if @critical < 13
    sequence << "m[#{Format.modifier(@first_modify)}]" if @first_modify != 0
    sequence << "m[#{@first_to}]" if @first_to != 0
    sequence << "r[#{@rateup}]" if @rateup != 0
    sequence << "gf" if @greatest_fortune
    sequence << "a[#{Format.modifier(@kept_modify)}]" if @kept_modify != 0

    sequence << Format.modifier(@modifier) if @modifier != 0

    return sequence.join
  end
end

RatingParsedに値を範囲内に収める正規化処理のメソッドを用意する。これは構文解析後に1回行えばよい。

# rating_parsed.rb
class RatingParsed
  def normalize!
    # クリティカル値を3以上に設定する
    @critical = (@critical || (half ? 13 : 10)).clamp(3..)
    # ...

    self
  end
end

# SwordWorld.rb
class SwordWorld < Base
  def rating(string) # レーティング表
    command = rating_parser.parse(string).normalize!
    # ...
  end
end

構文解析器では、#parsed において、オプションが設定されたかを if で判定するように変更する必要があるが、構文解析中の判定にも重複箇所が多くあるため、専用のBuilderを作るのがよいかもしれない。#515 の変更にも合わせる。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring 内部構造の改良
Projects
None yet
Development

No branches or pull requests

1 participant