4
\$\begingroup\$

I've been using Ruby for about a year now, mostly for small scripts with uses ranging from data munging to DevOps tasks. However, I am not intimately familiar with OOP (I appreciate the functional programming approach I'm often able to take with Ruby) and so I think my code is harder to read and debug than it could be.

I do not think my code is idiomatically Ruby, and I think I'm using a procedural style. I've included a recently-written Ruby script as an example, but at what point should one refactor into a OOP style? Is it always helpful to begin with a OOP structure? And I mean helpful in two senses: the sense that it will be easier for other team members to read the code; and also in the sense that it will make the program more robust and easier to debug.

The script below was intended as a small auditing tool. It takes a CSV (in two possible formats) and compares it to a database, but it has to do a lot of munging and matching because the results are not easily directly comparable.

#!/usr/bin/env ruby
require 'csv'
require 'date'
require 'json'
require 'mysql2'
require 'pp'
# Command line argument handling
if ARGV.length < 2
 puts "Usage: <CSV path or filename> <User's screenname>"
 puts "Outputs JSON of results we do not have in our database"
 exit
end
ARGV.each do |arg|
 # If the arg is a file that exists...
 File.file?(arg) ? @file = arg : @user = arg
end
def redacted?(filename)
 /redacted/.match(filename)
end
def make_redacted(csv)
 header = csv.slice!(0).map {|x| x.split.join.downcase.gsub(/\(\$\)/, '').to_sym}
 fd_csv = csv.map {|row| Hash[header.zip(row)]}
 fd_csv.map do |row|
 row[:name] = row[:title]
 row[:hashtag] = row[:sport]
 row[:opponent_score] = row[:oppscore]
 row[:place_finished] = row[:position].to_i
 row[:prize] = row[:winnings].to_f
 row[:date] = Date.parse(row[:date]).strftime('%Y-%m-%d')
 #row[:opponent_name] = row[:opponent]
 row[:score] = row[:score].to_f
 row[:entrants] = row[:entries].to_i
 row[:buy_in] = row[:entry].to_f
 row.delete_if { |k,v| [:title, :entries, :sport, :oppscore, :position,
 :opponent].include? k }
 end
 end
 def make_other_redacted(csv)
 ## Same sort of deal, except mapping other fields
 return dd_csv
end
# Read the database into memory too
def our_results
 # Set site ID to magic int
 site_id = redacted?(@file) ? 2 : 20
 db_hash = []
 client = Mysql2::Client.new(:host => "hostname",
 :username => "etc",
 :password => "etc",
 :port => etc,
 :database => "etc"
 )
 sql = "long sql query with some #{string_interpolation}"
 # Had to turn off casting due to some issues in the DB results
 db_results = client.query(sql, :symbolize_keys => true, :cast => false).each do |row|
 # Here I'm mapping hash names again
 row[:score] = row[:score].to_f
 # etc...
 db_hash << row
 end
 db_hash
end
# Takes a single object and an array
def match_redacted?(source, target)
 target.detect{|x| source[:prize] == x[:prize] &&
 # Match based on a list of hash values matching 
 }
end
def match_other_redacted?(source, target)
 target.detect{|x| source[:prize] == x[:prize] &&
 # Same thing, different matching criteria
 }
end 
def get_remainder(source, target)
 if redacted?(@file)
 source.delete_if {|s| match_redacted?(s, target)}
 else
 source.delete_if {|s| match_other?(s, target)}
 end
end
db_results = our_results()
# Read the csv into memory, fix it up
csv = CSV.read(@file)
source = fanduel?(@file) ? make_redacted(csv) : make_other_redacted(csv)
# Delete the results from the source that we match in the DB
remainder = get_remainder(source, db_results)
unless remainder.empty?
 puts JSON.generate(remainder)
end
asked Jun 26, 2015 at 17:29
\$\endgroup\$
4
  • 1
    \$\begingroup\$ The desire to improve code is implied for all questions on this site. Question titles should reflect the purpose of the code, not how you wish to have it reworked. See How to Ask. \$\endgroup\$ Commented Jun 26, 2015 at 18:00
  • \$\begingroup\$ Thanks for the link. It doesn't seem like this question is able to be asked on this site. \$\endgroup\$ Commented Jun 26, 2015 at 20:15
  • 1
    \$\begingroup\$ Don't misunderstand: The question's fine for this site in the sense that there is reviewable code there. However, if you only want to know "how do I make this object-oriented?" then it's a straight-up "how do I...?"-question and not really a request for a review. A review can touch on any aspect of the code, so a valid review could just look at the code you have and recommend improvements without ever discussing object-orientation - or even advise against it. Or it could do the other thing, and even provide new code. \$\endgroup\$ Commented Jun 26, 2015 at 21:39
  • \$\begingroup\$ You do not need OOP. \$\endgroup\$ Commented Jun 27, 2015 at 19:37

1 Answer 1

3
\$\begingroup\$

Never use OOP just because people say it's cool. If you do not know how to apply it, that means you just do not need it in this task. That's it. Forget about OOP. Involving it without much need only makes code worse and unreadable.

answered Jun 27, 2015 at 19:38
\$\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.