CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3
AnApprentice
AnApprentice

Reputation: 111030

Rails 3 - How to deal with complicated Switch Statements / If Statements

I'm building a method that ingests incoming email, and processes the email. Along the way there are a lot of things that could prevent the email from being processes successfully. The wrong reply-to address, the wrong from address, an empty message body etc..

The code is full of Switch Statements (case/when/end) and If statements. I'd like to learn a smarter, cleaner way of doing this. Additionally, a way to can track an error and at the end have one location where it emails back the user with an error. Is something like this possible with rails?

@error = []

Case XXX
when xxxx
  if XXXXX
  else
   @error = 'You don't have permission to reply to the xxxxx'
  end
else
  @error = 'Unfamilar XXXX'
end

Then something at the end like...

If @errors.count > 0
   Send the user an email letting them know what went wrong
else
 do nothing
end

Thanks for the help here. If you know of any other tutorials that would teach me how to write logic like the above smarter, that'd be great. Right now I have case/if statements going 3 levels deeps, it's hard to keep it straight.

Thanks

Upvotes: 1

Views: 687

Answers (4)

moeffju
moeffju

Reputation: 4403

I suggest using Exceptions. Start with this tutorial, then use Google, trial and error to go from there.

Edit: In more complex cases, exceptions may not be the right tool. You might want to use validator functions instead, for example (see other answers), or you could just return early instead of nesting ifs, e.g.:

unless sender_valid?
  @error = "Sender invalid"
  return
end
unless subject_valid?
  @error = "Invalid command"
  return
end
# normal no-errors flow continues here...

Upvotes: 2

klew
klew

Reputation: 14977

First, I would just assign a symbol to each error message as a simple hash:

ErrorsDescription = {
  :first => "First error",
  :second => "Second error",
  ...
}

And use symbols instead of strings.

Then, your if and switch statements. Basicaly I can't really help you, because I don't see what kind of condition statements you have. What are you checking? Why do you have 3 level deep conditions? Probably you can write it simpler using if and switch - so this is my first answer to this issue. Another solution may be writing simple methods to improve readability, so you can write like this:

if @email.has_wrong_reply_to_address? 
  @errors << :wrong_reply_to_address
else
  ...
end

Also, as @mpapis suggested, you can use Rails build in validation system, but not as ActiveRecord but as ActiveModel. Here you have some examples how to do it and how it works (also take a look here). Of course you may need to write custom validations, but they are just simple methods. Once you do all above job, you can just use:

@email.valid?

And if it is not, you have all errors in hash:

@email.errors

Just as in ordinary ActiveRecord object.

Then you may extend your Emial class with send_error_email method which sends an email if there was an error.

EDIT:

This is about new information you attached in comment.

You don't have to use nested ifs and switch here. You can have it looking like this:

def is_this_email_valid?
  if !email_from_user_in_system?
    @errors << :user_not_in_system
    return false
  end
  if comment_not_exists?
    @errors << :comment_not_exists
    return false
  end
  if user_cannot_comment_here?
    @errors << :permision_error
    return false
  end
  ...
  true
end

Then you can use it:

if [email protected]_this_email_valid?
  @email.send_error_mail
end

Upvotes: 3

mpapis
mpapis

Reputation: 53178

Is it possible to map your message to a model ? then all the if/switch logic would be validations and automatically handled by rails. Good starting point is active record validations guide

Also worth reading is action mailer guide

Upvotes: 1

zsalzbank
zsalzbank

Reputation: 9857

You could throw an error when something is not right. Then catch it at the end of your method.

http://phrogz.net/programmingruby/tut_exceptions.html

To make your code more readable and not have a lot of switch and if/then statements, you could create separate methods that validate certain aspects and call them from your main error-checking method.

Upvotes: 1

Related Questions