Skip to content

Instantly share code, notes, and snippets.

@seeflanigan
Created July 2, 2016 20:48
Show Gist options
  • Save seeflanigan/87d3a2dec831f06ce5a316fee2fbb12a to your computer and use it in GitHub Desktop.
Save seeflanigan/87d3a2dec831f06ce5a316fee2fbb12a to your computer and use it in GitHub Desktop.
def attrs
@attrs ||= Reading.new.attributes.keys.take(5).last(4)
end
def data
@data ||= File.read("tmp/output_small_test.csv")
end
def rows
data.split()
end
def cells
rows.map {|r| r.split(",") }
end
def hashes
cells.map do |row|
attrs.zip(row).
map {|lbl, val| {lbl.to_sym => val }}.
reduce(&:merge)
end
end
def readings
hashes.map {|r| Record.new(r) }
end
@JoshCheek
Copy link

My criterion tends to be immutability, but I think a lot of people would assert that all data should be passed as args. There's a certain amount of wiggle room based on you define things, but prob not for the csv filename. I'd expect that wrapping it in ImportRecordsFromCSV.call(csv_path) would be more in line with the paradigm. That toplevel call method will be responsible for conducting the flow of data through any other methods you use. Another heuristic is that callstacks are better when shallow and wide than when deep and narrow. Here, reading calls hashes calls cells calls rows calls data. So probably readings is your ImportRecordsFromCSV.call method, try moving the integration points up to there. Eg you can take hashes out by calling it like this: hashes(cells, attrs) I tend to find that the more I do this, the more procedural my code looks, because at some point, you give it what it needs, and it just does one thing, and then its like "well, I could have just done that thing, so why bother with a method?" so then I inline it. Some people are critical of code that looks this way, but I've had fewer issues with it, it seems to tolerate volatility well. IMO, both local variables and methods are just ways to assign a name.

A few things to look out for:

  • Hashes can prob be simplified with .to_h, eg [:a,:b,:c].zip([1,2,3]).to_h # => {:a=>1, :b=>2, :c=>3}
  • The relative file path in line 2 can potentially break. You're prob fine b/c you're prob running via a Rake task or something, and Rake resets the CWD. But could hit issues outside that environment, depending on how you call it / where you call it from. The best way I've found for dealing with this is File.expand_path(path_relative_to_file_dir, __dir__)
  • The way Reading finds its attrs is pretty fragile (eg if you add certain columns to the db, this probably breaks)
  • It's a little iffy to name the file with "records" when it is returning Reading objects.

@JoshCheek
Copy link

Oh, maybe you transcribed it into this gist and changed some names? Otherwise, its not clear to me why attrs are pulled from Reading, but then assigned to Record.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment