1
\$\begingroup\$

I'm a little unhappy with the look of this "flexible" way of dealing with errors from an API in Ruby:

class ApiError < StandardError
 def initialize(response)
 message = response.is_a?(String) ? response
 : !(response.respond_to? 'body') ? 'Error'
 : response.body.is_a?(Hash) ? response.body['message']
 : response.body.to_s
 super(message)
 end
end

The idea is you can pass to ApiError.new either a string, or an object with an accessor called body in which case we'll take the body if it's a string or its message property if it's a hash. We'll use a generic error message if the argument is anything else.

What would be the idiomatic alternative, if any? I'm happy to take suggestions for completely different approaches (multiple constructors, static factories, etc.) but am still interested in a single-initialize-method approach. I know I can do if...elsif...elsif...else...end. Is that better?

asked Oct 12, 2016 at 19:43
\$\endgroup\$
1
  • \$\begingroup\$ I get a syntax error with your code. Maybe some brackets help. \$\endgroup\$ Commented Oct 12, 2016 at 20:15

1 Answer 1

3
\$\begingroup\$

I would try a sequence of case statements:

class ApiError < StandardError
 def initialize(response)
 message = case
 when response.is_a?(String)
 response 
 when !(response.respond_to? 'body')
 'Error'
 when response.body.is_a?(Hash)
 response.body['message']
 else
 response.body.to_s
 end
 super(message)
 end
end
###test code
class XX
 attr_accessor :body
end
p ApiError.new('string') 
p ApiError.new(:x) 
xx = XX.new()
p ApiError.new(xx)
xx.body={'message' => 'my message'}
p ApiError.new(xx)

I hope I understood your code correct. Actually there is no check, if a given Hash has a 'message'-key.

You can also omit the message variable:

class ApiError < StandardError
 def initialize(response)
 super case
 when response.is_a?(String)
 response 
 when !(response.respond_to? 'body')
 'Error'
 when response.body.is_a?(Hash)
 response.body['message']
 else
 response.body.to_s
 end
 end
end
answered Oct 12, 2016 at 20:26
\$\endgroup\$
2
  • \$\begingroup\$ Agreed that the message-key check is good... when response.body.is_a?(Hash) && response.body.has_key?('message') \$\endgroup\$ Commented Oct 12, 2016 at 20:37
  • \$\begingroup\$ Good answer, just two comments. 1) This is not the idiomatic case indentation. IMO it's ok to use it, though, maybe adding a note? 2) Mixing calls with parens and calls without parens does not look nice. \$\endgroup\$ Commented Oct 13, 2016 at 7:29

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.