Creating a pure Ruby object (PORO) to email files in a Rails application
Good job on


Tightly focused methods that do what their name says
Non-surprising formatting
Testable design


This is pretty good code

Environment variable

There's a few things that bother me about this class's use of the S3_BUCKET environment variable.


It makes unit test awkward, as the test must either set the environment variable and then restore it, or it must stub ENV#[], either of which is a minor pain.
It hard-codes the environment variable name


My first thought was to have the class take the bucket name as an argument with a default:

def initialize(files, params, bucket_name = ENV['S3_BUCKET'])


But I think that is a half measure.  Instead, let's just pass in the bucket:

def initialize(files, params, bucket)


bucket is an instance of this class, which does a lazy fetch of the AWS::S3 bucket:

require 'forwardable'
require 'memoist'

class Bucket

  extend Forwardable

  def initialize(bucket_name = ENV['S3_BUCKET'])
    @client = AWS::S3.new
    @bucket_name = bucket_name
  end

  def_delegator :bucket, :objects

  private

  def bucket
    @client.buckets[@bucket_name]
  end
  memoize :bucket

end



The memoist gem Is a fancy version of "@bucket ||= ...".
Forwardable#def_delegator delegates the #object method to the result of the #bucket method.


The use of the Bucket class has these advantages:


FileSharingService:

no longer has to know about the environment variable.
does not have to concern itself with lazy-initialization and caching.
can now be easily tested in isolation--just give it a test double for its bucket instance.

Bucket can be easily tested in isolation.


Checking email addresses

I would consider taking the regular expression in this expression:

email !~ /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i


And, at a minimum, give it a constant.  The goal is to make the code more self-documenting:

WELL_FORMED_EMAIL_ADDRESS = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i
...
email !~ WELL_FORMED_EMAIL_ADDRESS


I might go one step farther and move the entire check into another class.  Make it someone else's responsibility to know what's a good email address and what's not:

EmailAddress.well_formed?(email)


params

The only use the class makes of params is of @params[:emails].  If that is unlikely to change in the future, I would consider just passing in the email addresses, and not concerning this class with params at all.

def initialize(files, email_addresses, ...)