Ruby 代码评论

Question

我是一名 Ruby 新手(4 天前开始，嘿，押韵!)并决定编写一个简单的小工具来娱乐/学习。以下代码是结果。它运行良好，但我非常感谢一些更有经验的 Ruby 开发人员的批评。我正在寻找关于风格、冗长和任何杂项的评论。提示/技巧。

顺便说一下，我对如何重写 lambda 递归更优雅很感兴趣。 (ctrl-f 用于“lambda 递归”)

注意:这不是 Ruby On Rails 项目。这是 Eve Online 的一个小工具。

require 'rubygems'
require 'reve'
require 'yaml'

BASE_SKILLPOINTS = [0, 250, 1414, 8000, 45255, 256000]

def calc_remaining_sp(rank, from_level, to_level)
  sp = BASE_SKILLPOINTS.map { |x| x * rank }
  [sp[to_level] - sp[from_level], 0].max
end

def summarize_skills(user_id, api_key)
  spec = YAML::load(File.open('ukc_skillsets.yaml'))

  api = Reve::API.new user_id, api_key

  #skill id => general skill info
  skills_list = Hash[api.skill_tree.map { |x| [x.type_id, x] }]


  api.characters.each { |char|
    char_sheet = api.character_sheet :characterID => char.id

    puts ""
    puts "Character - #{char.name}"
    puts "-------------------------------------------------"

    char_skills = Hash[skills_list.map { |id, skill| [skill.name, {:level => 0, :remaining_sp => [], :info => skill}] }]

    char_sheet.skills.each do |skill|
      skill_name = skills_list[skill.id].name
      char_skills[skill_name][:level] = skill.level
    end

    #compute the sp needed for each skill / each level
    char_skills.each_pair do |outer_skill_name, *|
      #lambda recursion
      calc_prereq_sp = lambda do |skill_name|
        prereq_remaining_sp = char_skills[skill_name][:info].required_skills.inject(0) do |sum, prereq_skill|
          prereq_skill_name = skills_list[prereq_skill.id].name
          #call the lambda
          calc_prereq_sp.call prereq_skill_name
          sum + char_skills[prereq_skill_name][:remaining_sp][prereq_skill.level]
        end
        current_skill = char_skills[skill_name]
        (0..5).each do |target_level|
          char_skills[skill_name][:remaining_sp][target_level] =
                  (calc_remaining_sp current_skill[:info].rank, current_skill[:level], target_level) +
                          prereq_remaining_sp
        end
      end
      calc_prereq_sp.call outer_skill_name
    end

    results = {}

    spec.each_pair do |skillset_name, *|
      process_skillset =  lambda do |name|

        details = spec[name]
        #puts "#{results} , name = #{name}"
        if results.include?(name) == false
          #puts "#{details['Prerequisites']}"
          remaining_sp = 0
          remaining_sp += details['Prerequisites'].inject(0) { |sp_remaining, prereq|
            process_skillset.call prereq
            sp_remaining + results[prereq][:remaining_sp]
          } if (details.include? 'Prerequisites') && (details['Prerequisites'] != nil)
          details['Required skills'].each_pair { |required_skill, target_level|
            remaining_sp += char_skills[required_skill][:remaining_sp][target_level]
          } if (details.include? 'Required skills') && (details['Required skills'] != nil)
          results[name] = {:remaining_sp => remaining_sp}
        end
      end
      process_skillset.call skillset_name
    end
    results.reject {|x| (spec[x].include? 'Private') && (spec[x]['Private'] == true)}.to_a().sort_by {|x|x[1][:remaining_sp]} \
      .each { |x, y| puts "#{x} = #{y[:remaining_sp]} sp left\n" }
  }
end
#userid, apikey hidden for confidentiality
summarize_skills 'xxxxxxx', 'yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy'

这是“ukc_skillsets.yaml”的一小段

Basics:
    #private = don't show at the end
    Private: True
    Required skills:
        #skill name: skill level
        Electronics: 4
        Engineering: 4
        Weapon Upgrades: 1
        Energy Systems Operation: 1
Armor Ship:
    Private: True
    Prerequisites:
        - Basics
    Required skills:
        Hull Upgrades: 4
        Mechanic: 4
......

Answer 1

我注意到的一些事情:

• 使用 block 时，惯用的做法是对单行使用 { ... }，对多行使用 do ... end。您倾向于将它们混为一谈。

• if results.include?(name) == false 读起来会比 unless results.include?姓名

• if (details.include? 'Prerequisites') && (details['Prerequisites'] != nil) 可以缩短为 if details['Prerequisites'] 如果您不明白原因，请阅读 Ruby 中的“真实性”。同样，results.reject {|x| (spec[x].include? 'Private') && (spec[x]['Private'] == true)} 可以变成 results.reject { |x| spec[x]['Private'] }

• 我认为嵌套的 lambda 情况有点乱，看起来可以提取到几个方法中。不过，如果不进行一些测试，我也不会开始尝试解开它。

• 尝试使用更具描述性的变量/方法名称。例如，calc_remaining_sp 可以用简单的 remaining_skillpoints 更好地表示。

一般来说，更多行更简洁的代码比更少行数更密集的代码更可取。示例:

results.reject {|x| (spec[x].include? 'Private') && (spec[x]['Private'] == true)}.to_a().sort_by {|x|x[1][:remaining_sp]} \
  .each { |x, y| puts "#{x} = #{y[:remaining_sp]} sp left\n" }

这可以重写为:

results.reject!  { |skill| spec[skill]['Private'] }
results.sort_by! { |skill| skill[1][:remaining_sp] } # sort_by! requires 1.9 - for 1.8 use results = results.sort_by...
results.each     { |x, y| puts "#{x} = #{y[:remaining_sp]} sp left\n" }

最后，当您开始编写更多方法时，请尝试为它们提供易于使用的 API。比如剩余技能点的方法:

def calc_remaining_sp(rank, from_level, to_level)
  sp = BASE_SKILLPOINTS.map { |x| x * rank }
  [sp[to_level] - sp[from_level], 0].max
end

如果其他人想要使用此方法，他们将不得不记住您在方法名称中使用的缩写以及参数应该出现的顺序，这并不好玩。一个更像 ruby 的替代方案是:

def remaining_skillpoints(options)
  skillpoints = BASE_SKILLPOINTS.map { |x| x * options[:rank] }
  difference  = skillpoints[options[:to]] - skillpoints[options[:from]]
  [difference, 0].max
end

这会让另一个 Rubyist 做类似的事情:

points_needed = remaining_skillpoints :rank => 6, :from => 3, :to => 5

或者，使用 Ruby 1.9 中新的 Hash 语法，它可以变得更加令人愉快:

points_needed = remaining_skillpoints rank: 6, from: 3, to: 5

但无论如何，四天后你的表现比我在 Ruby 上的表现要好得多。如果这段代码是您计划在未来构建的东西，我肯定会尝试在您开始重构之前添加一些测试。查看RSpec . Ruby Best Practices 的第 1 章对其进行了很好的介绍。 (免费 PDF)。

享受 Ruby!