0

我知道这段代码不是最优的，关于如何改进它的任何想法？

job_and_cost_code_found = false
  timberline_db['SELECT Job, Cost_Code FROM [JCM_MASTER__COST_CODE] WHERE [Job] = ? AND [Cost_Code] = ?', job, clean_cost_code].each do |row|
    job_and_cost_code_found = true
  end

if job_and_cost_code_found == false then 
  info = linenum + "," + id + ",,Employees default job and cost code do not exist in timberline. job:#{job} cost code:#{clean_cost_code}"
  add_to_exception_output_file(info)
end
4

2 回答 2

2

你在这里打破了很多简单的规则。

不要选择你不使用的东西。

您选择了许多列，然后完全忽略结果数据。你可能想要的是一个计数：

SELECT COUNT(*) AS cost_code_count FROM [JCM_MASTER__COST_CODE] WHERE [Job] = ? AND [Cost_Code] = ?'

然后你会得到一个row值为零或非零的值。将其保存到变量中，例如：

job_and_cost_codes_found = timberline_db[...][0]['cost_code_count']

不要比较，false除非你需要区分它和nil

在 Ruby 中，只有两件事评估为false和. 大多数情况下，您不会担心差异。在极少数情况下，您可能希望对 set 、 set或 not set ( ) 有不同的逻辑，然后才需要专门进行测试。nilfalsetruefalsenil

但是，请记住，这0不是一个错误值，因此您需要与之进行比较。

考虑到之前的优化，您if可能是：

if job_and_cost_codes_found == 0
  # ...
end

不要使用then或其他多余的语法

大多数 Ruby 风格指南摒弃了无用的语法then，就像他们建议避免for使用更灵活的 Enumerable 类一样。

操作数据，而不是字符串

您最终在那里组装了某种类似 CSV 的行。理想情况下，您将使用内置的 CSV 库进行正确的编码，并且像这样的库需要数据，而不是他们必须解析的字符串。

更接近这一点的是：

line = [
  linenum,
  id,
  nil,
  "Employees default job and cost code do not exist in timberline. job:#{job} cost code:#{clean_cost_code}"
].join(',')

add_to_exception_output_file(line)

您可能会替换join(',')为适用于此处的正确 CSV 编码方法。当您可以提前将所有数据编译成数组数组时，该库会更高效，因此如果这是最终目标，我建议您这样做。

例如：

lines = [ ]

# ...

if (...)
  # Append an array to the lines to write to the CSV file.
  lines << [ ... ]
end

将您的数据保存在标准结构中，例如数组、哈希或自定义对象，直到您准备好将其提交为其最终格式化或编码形式。这样，如果您需要执行过滤等操作，您可以对其执行其他操作。

于 2013-06-20T06:34:32.480 回答
0

当我不确定它应该做什么时，很难重构它，但是假设您想在没有匹配作业和代码对的条目时记录错误，这就是我想出的：

def fetch_by_job_and_cost_code(job, cost_code)
  timberline_db['SELECT Job, Cost_Code FROM [JCM_MASTER__COST_CODE] WHERE [Job] = ? AND [Cost_Code] = ?', job, cost_code]
end

if fetch_by_job_and_cost_code(job, clean_cost_code).none?
  add_to_exception_output_file "#{linenum},#{id},,Employees default job and cost code do not exist in timberline. job:#{job} cost code:#{clean_cost_code}"
end
于 2013-06-20T06:36:45.273 回答