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?
-
\$\begingroup\$ I get a syntax error with your code. Maybe some brackets help. \$\endgroup\$knut– knut2016年10月12日 20:15:06 +00:00Commented Oct 12, 2016 at 20:15
1 Answer 1
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
-
\$\begingroup\$ Agreed that the message-key check is good...
when response.body.is_a?(Hash) && response.body.has_key?('message')
\$\endgroup\$Ray Toal– Ray Toal2016年10月12日 20:37:49 +00:00Commented 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\$tokland– tokland2016年10月13日 07:29:16 +00:00Commented Oct 13, 2016 at 7:29