Masking user input passwords

Question

I have a method that accepts user input. Additionally, it masks passwords then the input is a password. It also bypasses the prompt of if an env variable supplies the needed input.

Apparently, my code needs refactoring, but how?

def self.gets(args)
  array = []
  args.each do |arg|
    print "#{arg.to_s.capitalize}: "
    if ENV['AWESOMEAPP_'+arg.to_s.upcase]
      array.push ENV['AWESOMEAPP_'+arg.to_s.upcase]
    else
      if arg.to_s=~/password/
        array.push STDIN.noecho(&:gets).chomp
      else
        array.push STDIN.gets.chomp
      end
    end
    print "\n"
  end
  array
end

Answer 1

After reading @user1777136 s answer, I came up with this

module Awesome
  # Has methods that are similar to but extended versions                                                        
  # of core ruby methods                                                                                         
  module Extensions
    # Get multipe inputs from user                                                                               
    # Masks passwords if detected                                                                                
    # Bypasses prompt when ENV variables present                                                                 
    def self.mgets(names)
      names.map do |name|
        sgets(name)
      end
    end

    # single gets                                                                                                
    def self.sgets(name)
      print "#{name.to_s.capitalize}: "
      return env_sgets(name) if ENV['AWESOME_' + name.to_s.upcase]
      return secure_sgets if name.match?(/password/)
      STDIN.gets.chomp
    end

    # get from env variable                                                                                      
    def self.env_sgets(name)
      ENV['AWESOME_' + name.to_s.upcase]
    end

    # secure gets from stdin                                                                                     
    def self.secure_sgets
      STDIN.noecho(&:gets).chomp
    end
  end
end

Answer 2

Granted I know nothing about the inputs and so on. But I see at least two responsibilities of the method:

1. Printing something
2. Returning an array of something

You might want to split the loop accordingly, which could give you:

def self.gets(args)
  array = []
  args.each do |arg|
    print "#{arg.to_s.capitalize}: "
    print "\n"
  end
  args.each do |arg|
    if ENV['AWESOMEAPP_'+arg.to_s.upcase]
      array.push ENV['AWESOMEAPP_'+arg.to_s.upcase]
    else
      if arg.to_s=~/password/
        array.push STDIN.noecho(&:gets).chomp
      else
        array.push STDIN.gets.chomp
      end
    end
  end
  array
end

This can help us further down the line. Nested if, if, else can be refactored to elsif. Rubocop can also tell you this. So the following will remove one level of nesting:

def self.gets(args)
  array = []
  args.each do |arg|
    print "#{arg.to_s.capitalize}: "
    print "\n"
  end
  args.each do |arg|
    if ENV['AWESOMEAPP_'+arg.to_s.upcase]
      array.push ENV['AWESOMEAPP_'+arg.to_s.upcase]
    elsif arg.to_s=~/password/
      array.push STDIN.noecho(&:gets).chomp
    else
      array.push STDIN.gets.chomp
    end
  end
  array
end

Furthermore it looks like we are always returning something from the second loop. So building a new "collector" array (which has a not so great name of array) can be avoided. Which gives us:

def self.gets(args)
  args.each do |arg|
    print "#{arg.to_s.capitalize}: "
    print "\n"
  end
  args.map do |arg|
    if ENV['AWESOMEAPP_'+arg.to_s.upcase]
      return ENV['AWESOMEAPP_'+arg.to_s.upcase]
    elsif arg.to_s=~/password/
      return STDIN.noecho(&:gets).chomp
    else
      return STDIN.gets.chomp
    end
  end
end

Since Ruby will return the last evaluated thing in the method and we are mentioning args twice now we can even chain this:

def self.gets(args)
  args
    .each { |arg| print "#{arg.to_s.capitalize}: \n" }
    .map do |arg|
      if ENV['AWESOMEAPP_'+arg.to_s.upcase]
        return ENV['AWESOMEAPP_'+arg.to_s.upcase]
      elsif arg.to_s=~/password/
        return STDIN.noecho(&:gets).chomp
      else
        return STDIN.gets.chomp
      end
    end
end

Finishing this off by extracting a method or two will lead us to:

def self.gets(args)
  args
    .each(&:print_arg)
    .map(&:read_arg)
end

def print_arg(arg)
  print "#{arg.to_s.capitalize}: \n"
end

def read_arg(arg)
  if ENV["AWESOMEAPP_#{arg.to_s.upcase}"]
    ENV["AWESOMEAPP_#{arg.to_s.upcase}"]
  elsif arg.to_s =~ /password/
    STDIN.noecho(&:gets).chomp
  else
    STDIN.gets.chomp
  end
end

Now I am pretty sure I have broken your code, but this can give you an insight into what you could do.