CODEWITHSUNDEEP
ruby-on-railsruby
krunal shah
krunal shah

Reputation: 16339

Proper way to write this condition

unless @client.nil? 
  TestMailer.snap_shot_error("test1","Errors",
      {:file_name => File.basename(upload_file),:client_name => @client.client_name})
else
  TestMailer.snap_shot_error("test1","Errors",
      {:file_name => File.basename(upload_file))
end

def snap_shot_error(to_address,subject,options={})
  # code
end

<%  if @client_name %>
  <%= _("There were problems with file ") + @file_name + _(" for client ") + @client_name %>
<% else %>
  <%= _("There were problems with file ") + @file_name %>
<% end %>

Upvotes: 1

Views: 73

Answers (1)

Ben Lee
Ben Lee

Reputation: 53319

For both of these questoin, you can use the ternary operator. It works like this

condition ? value_if_true : value_if_false

This is an expression (a sequence of values and operators that produces another value). It determines whether the condition is true or false, and evaluates to the first value (after the ? and before the :) if the condition is true, and the second value (after the :) if the condition is false.

So, for the first code example you posted, you can do this:

TestMailer.snap_shot_error("test1", "Errors",
  :file_name => File.basename(upload_file),
  :client_name => @client ? @client.client_name : nil)

[Note that I've remove the curly braces around the options -- in Ruby there are not necessary for the final options hash, and it is idiomatic to leave them off]

Or if for some reason you don't even want a nil :client_name in the hash, you can use the ternary operator and a merge:

TestMailer.snap_shot_error("test1", "Errors",
  {:file_name => File.basename(upload_file)}.merge(
    @client ? { :client_name => @client.client_name } : {}))

For the view, you can also use the ternary operator:

<%= _("There were problems with file ") + @file_name +
  (@client_name ? _(" for client ") + @client_name : '' ) %>

And now that I see what you are doing with @client_name, I don't see why you said you require that it not even be in the hash. The first code example I posted, where it passes ":client_name => @client.client_name" if there is a client and passes ":client_name => nil" if there is not a client, should work just fine. There's no reason to not pass :client_name instead of just passed a nil :client_name.

Upvotes: 3

Related Questions