8
\$\begingroup\$

I wrote this little script in PHP a bit ago. It takes a .csv file and outputs a .txt file with certain data arranged in a "psuedo-array" looking format. I recently started messing around with Ruby so I decided to redo it. It wound up essentially as an exact translation, though.

Is there a simpler / more Ruby way to do this?

require "csv"
class ArrayGenerator
 def initialize( file )
 @file = file
 open_csv
 end
 private
 def open_csv
 File.open("#{@file}" + "_output.txt", 'w') do |file|
 CSV.foreach(@file) do |row|
 lat = row[6]
 lon = row[7]
 img = "#{row[0]}".downcase
 part = "#{row[2]}"
 loc = "#{row[5]}"
 country = "#{row[4]}"
 focus = "#{row[3]}"
 desc = "#{row[8]}"
 link = "#{row[9]}"
 file.write("[#{lat}, #{lon}, #{img}, #{part}, #{loc}, #{focus}, #{country}, #{desc}, #{link}],
")
 end
 end
 end
end
ARGV.each do |file|
 g = ArrayGenerator.new( file )
end
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 7, 2012 at 15:21
\$\endgroup\$

1 Answer 1

10
\$\begingroup\$
img = "#{row[0]}".downcase
part = "#{row[2]}"
loc = "#{row[5]}"
country = "#{row[4]}"
focus = "#{row[3]}"
desc = "#{row[8]}"
link = "#{row[9]}"

The only difference between writing "#{ expression }" (without anything else in the string) and just writing expression is that the former converts the result of the expression to a string. If that is your intent, you should write expression.to_s instead because that way it's clearer what's going on.

However in your case the values in row are already string, so there's no need to convert them to strings, so using "#{}" here is just redundant. You can just write img = row[0].downcase, part = row[2] etc.

Instead of taking each column out of the row array individually, you can also write this:

CSV.foreach(@file) do |img, _, part, focus, country, loc, lat, lon, desc, link|
 img = img.downcase
 file.puts("[#{lat}, #{lon}, #{img}, #{part}, #{loc}, #{focus}, #{country}, #{desc}, #{link}],")
end

Note that I've used the variable name _ to denote a variable we're not going to use because you never used row[1] in your code (_ is conventionally used in Ruby to denote variables that aren't used and is in fact the only variable name that's allowed to appear multiple times in a parameter list).

I've also replaced write with puts which automatically inserts a newline after the string, so you don't have to include the newline in the string.


A couple of more general design notes:

I think open_csv is rather misleadingly named. It does a lot more than just opening the csv file.

I also think it's a bad idea to call open_csv in your constructor. Generally the constructor should only set up the object, so that it's in a usable state. It should not do any actual work. The way you designed it, everything the object is ever going to do is done in the constructor. That's why when you use it, you create the object with ArrayGenerator.new and then never do anything with the object - because everything that can be done with the object has already been in the constructor. Another sign of this is that the only public method in your class is initialize. This is a very strange way to use objects.

On a related note there's no point in creating a g variable if you're not going to use it.

You should either redesign your class to behave more like an actual class, with a usage more like this:

g = ArrayGenerator.new( file )
g.generate

Or you could get rid of your class altogether and instead define a stand-alone method that could be used like this:

generate_array( file )
answered Sep 7, 2012 at 15:57
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.