Using as few queries as possible for many table associations

Question

I have a table containing about 30k rows of data and there's several table associations. The problem is that there's like 18 columns, and each column requires 1 db query.

Here's an example of what I'm doing:

if col_num == 0
 tmp_string_text = finding_phase
elsif col_num == 1
 tmp_string_text = ""
elsif col_num == 2
 tmp_string_text = ""
elsif col_num == 3
 tmp_string_text = finding.vuln_finding.severity
elsif col_num == 4
 tmp_string_text = finding.node.ip
elsif col_num == 5
 tmp_string_text = finding.node.host_name
elsif col_num == 6
 tmp_string_text = finding.node.dns_name
elsif col_num == 7
 tmp_string_text = finding.port
elsif col_num == 8
 tmp_string_text = finding.pentest_finding.name
elsif col_num == 9
 tmp_string_text = finding.vuln_finding.name
elsif col_num == 10
 tmp_string_text = finding.vuln_finding.description
elsif col_num == 11
 tmp_string_text = finding.vuln_finding.solution
elsif col_num == 12
 tmp_string_text = finding.additional_output
elsif col_num == 13
 tmp_string_text = finding.cve
elsif col_num == 14
 tmp_string_text = finding.node.os
elsif col_num == 15
 tmp_string_text = finding.node.device_type
elsif col_num == 16
 tmp_string_text = finding.node.scan_time
end

As a result, there's going to be 15 queries to the DB per 1 row. Is there any way that I can make this more efficient? This takes a REALLY long time when I have 30k rows. Not looking for anyone to flame here, just looking for advice. Pretty new to Ruby on Rails but learning as I go.

Basically, "finding" in this case is a result of @my_report.findings and it's going through each finding and pulling back data.

• 200_success: Welcome to Code Review! As a courtesy to other users, please declare your cross-posts.
• 200_success: Stack Overflow can reformulate your code, but Code Review would prefer to give you the best possible advice for your situation. Please tell us more about what you are trying to accomplish. Is there a reason for this particular column numbering? What is tmp_string_text for? What is the finding object? What about finding_node_ip — is that a function call, a method call, or a variable? Better yet, show your entire function or class, so that we have more context.
• LewlSauce: Thanks. So tmp_string_text is the data that will go in that column. I'm iterating through a bunch of columns and if, for example, the column == 1, then tmp_string_text gets assigned that text. At the bottom of this function, I'm doing something with tmp_string_text (inserting it into a cell.
• 200_success: But your "explanation" just reiterates what the code says, and doesn't answer my questions.
• LewlSauce: Long story short, tmp_string_text holds data based on the colum number. finding_node_ip = finding.node.ip. Ultimately, as opposed to calling finding.node.os, finding.node.device_type, finding.node.scan_time, finding.vuln_finding.description, etc, I'm wondering if there's a way to call all of the required information in one call. Sorry. Let me know if that helps.

Answer 1

When I see long lists of if/elsif, I look to see whether case/when is a good alternate:

tmp_string_text = case col_num
 when 0
 finding_phase
 when 1, 2
 ""
 when 3
 finding.vuln_finding.severity
 when 4
 finding.node.ip
 when 5
 finding.node.host_name
 when 6
 finding.node.dns_name
 when 7
 finding.port
 when 8
 finding.pentest_finding.name
 when 9
 finding.vuln_finding.name
 when 10
 finding.vuln_finding.description
 when 11
 finding.vuln_finding.solution
 when 12
 finding.additional_output
 when 13
 finding.cve
 when 14
 finding.node.os
 when 15
 finding.node.device_type
 when 16
 finding.node.scan_time
 end

In this case I think it's a reasonable solution. If you were only returning strings then a hash would be even more compact:

tmp_string_text = {
 0 => 'finding_phase',
 1 => "",
 2 => "",
 3 => 'finding.vuln_finding.severity',
}[col_num]

That's obviously truncated, but you get the idea.

But, again, you're executing code for your return value once your condition matches. Trying to use a hash will cause all that code to execute when the hash is initialized, which is likely to not be what you want.

As an exercise, I tried clarifying what's going on by using nested case statements:

tmp_string_text = case col_num
 when 0
 finding_phase
 when 1 .. 2
 ""
 when 3
 finding.vuln_finding.severity
 when 4 .. 6
 meth = case col_num
 when 4
 :ip
 when 5
 :host_name
 when 6
 :dns_name
 end
 finding.node.send(meth)
 when 7
 finding.port
 when 8
 finding.pentest_finding.name
 when 9 .. 11
 meth = case col_num
 when 9
 :name
 when 10
 :description
 when 11
 :solution
 end
 finding.vuln_finding.send(meth)
 when 12
 finding.additional_output
 when 13
 finding.cve
 when 14 .. 16
 meth = case col_num
 when 14
 :os
 when 15
 :device_type
 when 16
 :scan_time
 end
 finding.node.send(meth)
 end

Obviously that's untested. It's a bit longer, but the nesting does help reveal the logic.

Then, as a DRY exercise, took that and tried to reduce the repetition a bit:

case col_num
when 0
 finding_phase
when 1 .. 2
 ""
when 3 .. 16
 result = finding
 methods = case col_num
 when 3
 [:vuln_finding, :severity]
 when 4 .. 6
 [:node] << case col_num
 when 4
 :ip
 when 5
 :host_name
 when 6
 :dns_name
 end
 when 7
 [:port]
 when 8
 [:pentest_finding, :name]
 when 9 .. 11
 [:vuln_finding] << case col_num
 when 9
 :name
 when 10
 :description
 when 11
 :solution
 end
 when 12
 [:additional_output]
 when 13
 [:cve]
 when 14 .. 16
 [:node] << case col_num
 when 14
 :os
 when 15
 :device_type
 when 16
 :scan_time
 end
 end
 methods.each do |m|
 result = result.send(m)
 end
 result
end

Again that's not tested. It loses some readability at that point but it's pretty easy to figure out what's going on.

• the Tin Man: You should read codereview.stackexchange.com/help/someone-answers.
• Bam: @theTinMan They look basically the same, except one is a line longer...
• the Tin Man: That's because the current starting code has little repetition in the tests. Note that 1, 2 have been combined where they can't be using if-based code. case/when has a lot of potential to reduce long conditional tests greatly.
• the Tin Man: Something to consider also is it's not the number of lines that's important, it's the readability and whether it's DRY enough. I'd much rather maintain some code that's a little verbose but very readable than code I have to spend 30 minutes decoding before I understand it.