重构这个红宝石数据库/续集宝石查找

Question

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

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 

Answer 1

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

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

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

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中区分只有两件事情作为评价假，nil和false。大多数时候你不会关心这种差异。在极少数情况下，您可能想要设置不同的逻辑集合true，设置为false或未设置（nil），然后才可以进行如此明确的测试。

但是，请记住0不是错误的值，因此您需要与其进行比较。

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

if job_and_cost_codes_found == 0 
    # ... 
end 

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

大部分红宝石风格导游摒弃无用的语法像then ，正如他们建议避开for，而是使用更灵活的Enumerable类。

处理数据，而不是字符串

你组装某种CSV样线到底有。理想情况下，你会使用built-in CSV library来做正确的编码，而像那样的库需要数据，而不是他们必须解析的字符串。

更近了一步，它是此：

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) 

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

例如：

lines = [ ] 

# ... 

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

请像数组，哈希，或自定义对象的标准结构数据，直到你准备把它提交到最终格式化或编码形式。这样，如果您需要执行过滤等操作，则可以对其执行其他操作。

Answer 2

很难重构这个时候我不完全相信它应该做的，但假设你希望在没有项目相匹配的作业&代码对记录的错误，这里是我想出什么附：

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