1
\$\begingroup\$

I have created a module that validates the credentials against different databases.

module Authentication

  DATABASES = %w[mysql mssql oracle].freeze

  DATABASES.each do |database|
    define_method("#{database}_connect") do |args|
      client = case database
               when 'mysql' then Mysql2::Client.new args.merge(ssl_mode: :disabled)
               when 'mssql' then TinyTds::Client.new args
               when 'oracle' then OCI8.new(args[:username], args[:password], "//#{args[:host]}:#{args[:port]}/#{args[:service_name]}")
               end
      case database
      when 'mysql', 'oracle' then true
      when 'mssql' then client.active?
      end
    end
  end

  def database_authentication_success?(database:, host:, port:, service_name:, username:, password:)
    options = {host: host, port: port, service_name: service_name, username: username, password: password}
    case database
    when 'mysql' then mysql_connect(options)
    when 'mssql' then mssql_connect(options)
    when 'oracle' then oracle_connect(options)
    end
  end
end

Please let me know, how can it be improved?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

What is the use case and rationale for this module? I can see what it does, but I'm struggling to understand why I'd want to do it.

The fact that you have three almost identical case statements to drive the behaviour is a 'code smell'. In most other OO languages you'd use inheritance or an interface and a factory method to determine which subclass of an object to create (with a single case statement). In that case, the objective is to abstract away the differences between connecting to the databases behind an interface (which is what you seem to be trying to do in method database_authentication_success?). But the metaprogramming code generates explicitly-named methods for connecting to each type of database, which seems to do the opposite and makes it harder to use.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.