2
\$\begingroup\$

I created a script to automate the task of uploading a CSV file to Box. I have a couple of files a Box client and a MySQL client. I create an instance of both on the function.rb file which is the one below. I wonder if there is anything to improve? if there are principles I'm not following or if I'm breaking best practices.

Is there anything that can be improved?

require 'json'
require 'date'
require 'dotenv/load'
require 'fileutils'
require 'csv'
require 'logger'
require './my_sql'
require './box_api'
require 'pry'
begin
 year = ARGV[0]
 month = ARGV[1]
 day = ARGV[2]
 search_timestamp = Time.new(year, month, day).utc
 db_client = MySQL.new(search_timestamp)
 emails = db_client.get_emails_from_db
 return 'No new emails found' if emails.entries.empty?
 box = BoxApi.new(ENV['BOX_USER_ID'])
 date = DateTime.now.strftime("%m-%d-%Y").to_s
 file_name = "access-emails-#{date}"
 CSV.open("./tmp/#{file_name}.csv", "wb") do |csv|
 emails.entries.each do |entrie|
 csv << [entrie.values[0], entrie.values[1]]
 end
 end
 box.upload_file_to_box("./tmp/#{file_name}.csv", file_name, ENV['BOX_FOLDER_ID'])
 FileUtils.rm("./tmp/#{file_name}.csv")
 puts "successfully uploaded CSV file to Box"
rescue StandardError => e
 logger = Logger.new(STDOUT)
 logger.level = ENV.fetch('LOG_LEVEL', Logger::INFO)
 logger.datetime_format = '%Y-%m-%d %H:%M:%S '
 logger.error("msg: #{e}, trace: #{e.backtrace.join("\n")}")
end
asked Jun 11, 2020 at 18:16
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I think your script would benefit from some object oriented composition. Basically you have three different concerns.

  • Config
  • File backup (select from database and storage in CSV)
  • File upload

A few things could change now, for instance backup from a different source (different database, cloud etc), upload to a different remote service (e.g. dropbox). Additionally, having small composable objects would make it easier to test this.

Here are some examples how to compose your script more object oriented.

Config

We could use an OpenStruct to store our config data. This way we only need to write our environment variables once, if we want to change them later there is only one place to update them.

require 'ostruct'
config = OpenStruct.new(
 year: ARGV[0],
 month: ARGV[1],
 day: ARGV[2],
 box_user_id: ENV['BOX_USER_ID'],
 box_folder_id: ENV['BOX_FOLDER_ID']
)

FileBackup

We can extract a backup file which just excepts rows and writes them to a CSV file. The dependency injection makes it also easier to test this (e.g. inject the data to write and the test directory)

class BackupFile
 def initialize(rows:, date: DateTime.now.strftime("%m-%d-%Y").to_s, directory: "./tmp")
 @rows = rows
 @date = date
 end
 def save
 CSV.open(full_path, "wb") do |csv|
 rows.each do |entry|
 csv << [entry.values[0], entry.values[1]]
 end
 end
 end
 def full_path
 File.join(directory, filename)
 end
 def delete
 FileUtils.rm(full_path)
 end
 private
 attr_reader :rows, :date
 def file_name
 "access-emails-#{date}"
 end
end
db_client = MySQL.new(search_timestamp)
emails = db_client.get_emails_from_db
return 'No new emails found' if emails.entries.empty?
file = BackupFile.new(emails.entries)
file.save

Upload

The uploader accepts a client, path and remote folder. Also notice that we have an adapter around the BoxApi to implement a common interface upload. If we want to swap it out to upload to Dropbox, we only need to write a DropboxClient adapter which we can inject into the uploader. To test, we can write even a TestClient.

class Uploader
 def initialize(client:, path:, remote_folder:)
 @client = client
 @path = path
 @remote_folder = remote_folder
 end
 def upload
 client.upload(path, file_name, remote_folder)
 end
 private
 attr_reader :client, :path, :remote_folder
 def file_name
 File.basename(path)
 end
end
class BoxClient
 def initialize(client:, box_user_id:)
 @client = client.new(box_user_id)
 end
 def upload(path, file_name, remote_folder)
 client.upload_file_to_box(path, file_name, remote_folder)
 end
 private
 attr_reader :client
end

Error handling

I would move the error handling into the classes directly and also inject the logger. Something like this:

class BoxClient
 def initialize(client:, box_user_id:, logger: Logger.new)
 @client = client.new(box_user_id)
 end
 def upload(path, file_name, remote_folder)
 client.upload_file_to_box(path, file_name, remote_folder)
 rescue BoxError =>
 logger.error("Upload failed: #{e.message}")
 end
 private
 attr_reader :client
end
answered Jun 11, 2020 at 20:26
\$\endgroup\$

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.