-2

I will admit that I'm still new to Ruby so I'm not yet familiar with the gotcha's and am still learning.

I've googled the issue many times but haven't been able to find an exact answer. Most results talk about "nested" if/else statements. Which isn't what I'm attempting. I found another answer from SO that talked about mapping the arrays based on conditions, but feel like that would bring me back to the same issues I'm already having.

Link to SO article regarding merging nested arrays if anyone is interested: Ruby: Merging nested array between each other depending on a condition

My Question:

While designing a simple CLI script in ruby using optparse to control the output based on conditionals. I came across an issue where I am unable to do multiple sequential if statements and concat/merge multiple arrays into one to be passed to another function.

For some reason only the last if block is being honoured. All if blocks before the last one return nil.

Can anyone tell me what I'm doing wrong and provide me with some relevant documentation regarding my issue. Such as why such an issue occurs?

Would love a working solution to learn from, but reference material would work as well since my goal is to learn from this issue.

Results of my tests:

$ ruby servers.rb
#=> ["server1", "server2", "server3"]
$ ruby servers.rb -m
#=> nil
$ ruby servers.rb -h
#=> nil
$ ruby servers.rb -s server6
#=> ["server6"]
$ ruby servers.rb -m -h
#=> nil

Below is my script:

#!/usr/bin/env ruby
require 'optparse'

@servers = {
  'primary' => %w[server1 server2 server3],
  'inhouse' => %w[server4 server5],
  'mlm' => %w[server6]
}

@options = {
  inhouse: false,
  server: [],
  mlm: false
}

# Parse options passed to medusabackup.rb
optparse = OptionParser.new do |opts|
  opts.on('-h', '--inhouse', 'Limit results to inhouse results') do
    @options[:inhouse] = true
  end
  opts.on('-m', '--mlm', 'Include mlm results') do
    @options[:mlm] = true
  end
  opts.on('-s', '--server=SERVER', 'Limit results to SERVER') do |f|
    @options[:server] << f
  end
  opts.on_tail('--help', 'Display this screen') { puts opts, exit }
end

begin
  optparse.parse!
rescue OptionParser::InvalidOption, OptionParser::MissingArgument
  puts $ERROR_INFO.to_s
  puts optparse
  exit
end

def selected_servers
  # Setup Selected variable as an array
  selected = []
  # If @options[:server]  array is not empty and @options[:server]
  # add @options[:server] servers to [selected] array
  if @options[:server].empty?
    # If @options[:mlm] is true add @server['mlm'] servers to [selected] array
    selected.concat @servers['mlm'] if @options[:mlm]
    # If @options[:inhouse] is true add @server['inhouse'] servers to [selected] array
    selected.concat @servers['inhouse'] if @options[:inhouse]
    # If @options[:mlm], @options[:inhouse] is true add @server['mlm'] servers to [selected] array
    selected.concat @servers['primary'] unless @options[:mlm] || @options[:inhouse]
  else
    selected.concat @options[:server]
  end
end
puts selected_servers.inspect

Thanks to Max and everyone for showing me my blunder. Forgot to return selected at the bottom of the function.

Synsa
  • 1
  • 2
  • Please check out the help section of the site to learn about the concept of a [Minimal Complete Verifiable Example](https://stackoverflow.com/help/mcve). Is this the shortest example that demonstrates your question? – pjs Mar 14 '19 at 20:44
  • 2
    This code does not behave like you show in fhe usage part. – Sergio Tulentsev Mar 14 '19 at 20:46
  • Hint: your `selected_servers` returns something, but not what you expect it to. – Sergio Tulentsev Mar 14 '19 at 20:50
  • @pjs -I apologize. I left an unused condition block in my code. Removed it. Reduced code to only the affected function. – Synsa Mar 14 '19 at 21:27
  • @Synsa: "reduced code" - yes, but now you reduced it _too much_. Now it's not runnable at all. Usage shows you specifying some command-line arguments, but parsing code is gone. For future question, aim to post code snippets that are ready for copy-paste-run-see (that's the "complete" and "verifiable" parts) – Sergio Tulentsev Mar 14 '19 at 21:48
  • 1
    @SergioTulentsev Sorry about that. (Making everyone happy is tough.) Added back the original optsparse code. Can confirm that as-is it is copy-paste-run'able. (Will learn for next time) – Synsa Mar 15 '19 at 02:02
  • @Synsa: great, thanks! – Sergio Tulentsev Mar 15 '19 at 07:47

1 Answers1

2

There's no explicit return in selected_servers, so it's returning the value of the last expression it runs, which is usually a failing unless. A failing if/unless returns nil.

Max
  • 21,123
  • 5
  • 49
  • 71
  • Isn't this a little misleading when the `if` in question is disguised as an `unless`? – mu is too short Mar 14 '19 at 20:59
  • 1
    @muistooshort: for me personally, `unless` always makes me stop and re-read the condition a few times. Is it really what I think it is? Really? Been burned by double negation a few too many times :) – Sergio Tulentsev Mar 14 '19 at 21:02
  • 1
    @SergioTulentsev Same here, I have to rewrite `unless cond` to `if !cond` in my head so I don't use it (except the `unless: :method_name` form with various Rails macros but even then I tend to write an extra method so that I can use `if:` instead). This particular case is even more insidious because there's a postfix `unless` inside an `if` and the overall expression ends up being the value of the method call. – mu is too short Mar 14 '19 at 21:21