makandra / makandra-rubocop

makandra's default Rubocop configuration
MIT License
6 stars 1 forks source link

Style of raise #4

Closed dastra-mak closed 5 years ago

dastra-mak commented 5 years ago

For me it was unexpected that Errors should not be instantiated with the message. In my opinion it's more common to raise with new.

-      raise WrongHeaderError.new(message)
+      raise WrongHeaderError, message

https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/RaiseArgs

Aspects to consider:

Should we raise with instantiation or not?

triskweline commented 5 years ago

If I have an error class with a custom constructor, will makandra-rubocop allow the instantiation?

dastra-mak commented 5 years ago

No, it will complain about

raise CustomError.new('message')

but not about

raise CustomError.new('message', :other)

Probably that's good enough for us.

foobear commented 5 years ago

For all I know, raise ErrorClass, error_message is the preferred way across the community.

If you want to raise a custom error that accepts other arguments than the message, the constructor is possibly different (maybe message is not the 1st argument) and can not be supported by raise. There are valid cases for this and you'd then say something like

raise IdentifierError.new('Record has different canonical identifier', identifier)

Rubocop will not complain about this, so I vote for keeping as is.

triskweline commented 5 years ago

Vote for keeping.

dastra-mak commented 5 years ago

I agree with keeping