Selecting CPUs and memory in Vagrant

I generally like to be Rubocop-clean, but it seems like breaking up the first case statement will only make things harder to read. Any suggestions?

Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  config.vm.provider 'virtualbox' do |vb|
    vb.customize ['modifyvm', :id, '--memory', host_hardware[:memory],
                  '--cpus', host_hardware[:cpus], '--ioapic', 'on']
  end

  private

  # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
  def host_hardware
    case RUBY_PLATFORM
    when /linux/
      cpus = `nproc`.to_i
      memory = `sed -n -e '/^MemTotal/s/^[^0-9]*//p' /proc/meminfo`.to_i / 1024
    when /darwin/ # OS X
      cpus = `sysctl -n hw.ncpu`.to_i
      memory = `sysctl -n hw.memsize`.to_i / 1024 / 1024
    when /cygwin|mswin|mingw|bccwin|wince|emx/ # Windows
      cpus = `wmic computersystem get numberofprocessors`.split("\n")[2].to_i
      memory = `wmic OS get TotalVisibleMemorySize`.split("\n")[2].to_i / 1024
    else 
      cpus = 2
      memory = 2048
    end
    { cpus: cpus, memory: reasonable_memory(memory) }
  end
  # rubocop:enable Metrics/AbcSize, Metrics/MethodLength

  def reasonable_memory(memory)
    half_of_memory = memory / 2
    case
    when half_of_memory  4096
      4096
    else
      half_of_memory
    end
  end
end

I'd start by breaking the host_hardware method into host_cpus and host_memory rather than returning a hash. A hash feels like a too "informal" structure.

Of course, you'll then need to do the case switching in multiple places, but you can streamline that a little bit by having a method that just returns a normalized platform identifier:

def host_platform
  case RUBY_PLATFORM
  when /linux/ then :linux
  when /darwin/ then :darwin
  when /cygwin|mswin|mingw|bccwin|wince|emx/ then :windows
  else :generic
  end
end

You'll still need case statements elsewhere, but at least they'll be simpler.

Alternatively, you can define the cpu/memory "getters" as procs or lambdas in a hash keyed by the platform:

# horrible name, but it's late
cpu_getters = {
  linux: -> { `nproc`.to_i },
  darwin: -> { `sysctl -n hw.ncpu`.to_i },
  windows: -> { `wmic computersystem get numberofprocessors`.split("\n")[2].to_i }
}

And a similar thing for getting memory information, of course.

Then, using the host_platform from before, you can so something like:

def host_cpus
  getter = cpu_getters[host_platform]
  getter ? getter.call : 2
end

And again, similar thing for memory.

Or, going all out, you could make "platform" classes with cpus and memory methods. If you need more than cpu and memory info,

Playing around a bit, I came up with the thing below. I know that is probably waaaay over-engineered for a simple vagrant file, but between one extreme and the other, you might find something useful.

module Platform
  def self.current
    case RUBY_PLATFORM
    when /linux/ then Linux.new
    when /darwin/ then Darwin.new
    when /cygwin|mswin|mingw|bccwin|wince|emx/ then Windows.new
    else Generic.new
    end
  end

  class Generic
    attr_reader :cpus, :memory

    def initialize
      @cpus = 2
      @memory = 2048
    end

    def reasonable_memory
      [1024, memory / 2, 4096].sort[1]
    end
  end

  class Linux < Generic
    def initialize
      @cpus = `nproc`.to_i
      @memory = `sed -n -e '/^MemTotal/s/^[^0-9]*//p' /proc/meminfo`.to_i / 1024
    end
  end

  class Darwin < Generic
    def initialize
      @cpus = `sysctl -n hw.ncpu`.to_i
      @memory = `sysctl -n hw.memsize`.to_i / 1024
    end
  end

  class Windows < Generic
    def initialize
      @cpus = `wmic computersystem get numberofprocessors`.split("\n")[2].to_i
      @memory = `wmic OS get TotalVisibleMemorySize`.split("\n")[2].to_i / 1024
    end
  end
end

With that you can call Platform.current and you'll get an object with the information you need. Note I've moved #reasonable_memory to an instance method too (and used a neat little trick to find the median value).

One obvious improvement would of course be memoizing Platform.current, so it only bothers with instantiating something once.

As a sidenote: I don't actually use Rubocop, though I generally adhere to the style guide, and agree with very nearly all of it (I often link to it in reviews, too). I tested all the code above though, and Rubocop let me off with a warning about line length (and a broken tail light).

But running Rubocop on a little script like this is somewhat overkill. Like getting out Strunk & White when writing a routine email. If you've used Rubocop for a while, I'm sure you've already learned to police your code yourself, and that's probably enough.

StackExchange Code Review Q#92960, answer score: 3