# Reduce columns to produce product keycodes

I have written the following code that achieves my desired goal of converting a "raw" CSV file into a keycoded list. The idea is that a person will get the first product they ask for, and English products are coded as blank because they are the default.

This gist contains a sample of the input and output data. As it stands, rubocop thinks my code is too complex, and I am inclined to agree with it here, as it feels like I am doing far more than I need to in order to get such a simple result.

Not sure if this is helpful but the rubocop stats I'm getting are as follows:

• ABC size too high (19.44/15)
• Cyclomatic complexity too high (9/6)
• Method has too many lines (17/10) (is this one really even a problem?)
• Perceived complexity too high (10/7)

Are any of these "true" problems? They seem to reflect my opinion that the code just turned out "ugly"

On paper it I narrowed it down to about 2 cases with some sub cases. (1. Normal header -> write 2.Header has "#" in it -> subcases) and in the second case it's either blank, "1", or a product name. If the line contains "English Map" I write nothing, otherwise I am to write the first product requested sequentially on the line.

require 'CSV'
in_file = 'test.csv'

def code_line_by_product(line)
new_line = []
tmp = nil
e_line = false
line.each do |item|
if item[0].include?('#')
if item[1] != ' ' && item[1] && item[1] != '1' && item[1] != ''
e_line = true if item[1].include?('English')
unless tmp || e_line
tmp = item[1][0]
new_line << tmp
end
end
else
new_line << item[1]
end
end
new_line
end

CSV.open("#{File.basename(in_file, '.csv')}_coded.csv", 'a+') do |out|
out << code_line_by_product(line)
end
end


Also, I've been noticing that I use that construction of writing from one file to another a lot and was wondering if there was a more convenient way of doing it (such as writing a function that takes the in file, out file, and the desired transformation). While it's only 4 lines of code, I essentially use those nested block all the time and it would be nice to only have to write the transformation function and then just call a single function to execute it.

• Unrelated to the actual code: Are those actual names and email addresses in the CSV? Because if it is, you'll want to remove it as fast as you possibly can! Apr 22, 2015 at 11:28
• Thank you for pointing that out! I John Smithed it. That could have turned into a very bad habit. Apr 22, 2015 at 13:19
• Great, but: gists are just git commits, so the original data are still there (click "Revisions"). I can't remember if there's an option to remove the gist revisions, or if it's easier to simply make a new gist. Either way, you should look into it. Apr 22, 2015 at 14:36
• Better. And yeah, just don't mess with user data. Ever, if possible. E.g. write some scripts to generate data for you (you'll end up doing that anyway if you write tests), or at least sanitize/anonymize real data before going near it. The latter is more dangerous, since stuff can slip through, or simply because there's no way to truly make it anonymous (cf. even metadata becomes actual data if you've got enough of it). Basically, handle user data like an epidemiologist handles a horrible virus: Wear tons of protective gear, have strict procedures, and don't let it out of the lab. Apr 22, 2015 at 18:41
• The main point is just to avoid exposing other people's data, so the more you do to prevent that, the less likely it is. As for testing, certainly look into that! Ruby coders are big on testing, so there's lots to find; Test::Unit is built in (stdlib), but there are many more tools and libraries. So it shouldn't be too hard to, for instance, write a few tests for your current code_line_by_product that verifies that its output is correct. Still, the final test is of course the real data and your eyes. Just don't post the results online :) Apr 22, 2015 at 21:42

I would do something like this. It is untested and may have typos but it should give you some idea of how to break the logic up. I would:

• Express variable to what they mean
• Use a class

Hope this helps

require 'CSV'

class CSVTranslator
def self.write file_path, options={}
new(file_path, options).translate
end

def initialize source_file, options={}
@source_file = source_file
@destination_file = options.fetch(:to) { "#{File.basename(source_file, '.csv')}_coded.csv" }
end

def translate
CSV.open(destination_file, 'a+') do |out|
each_row { |row| out << row }
end
end

private

end

def each_row
yield code_line_by_product(line)
end
end

def code_line_by_product(line)
new_line = {}
english_language = false
keycode = ""

english_language ||= value.include?('English')
keycode = value.first if value.first != '1'
else
end
end

new_line['keycode'] = keycode unless english_language || keycode.blank?

new_line
end
end

CSVTranslator.write 'test.csv', to: 'test_coded.csv'

• So if I'm getting the idea here, I was getting too caught up in mucking about with arrays when a hash could have handled a lot of the heavy lifting for me? Apr 22, 2015 at 18:19
• I still think there is a better way by being explicit in the headers. Will the source csv headers vary? Apr 22, 2015 at 18:25
• In this particular case I don't believe they will. Ideally I would just take the original headers, remove the ones containing "#" and then tack on the keycode header. That part where I'm explicitly creating the headers was kinda a lazy solution to what I'm now realizing could probably be done using the array deleteif method or whatever. Apr 22, 2015 at 18:39