Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing Header Raises Roo::HeaderRowNotFoundError #247

Merged
merged 2 commits into from
Aug 13, 2015

Conversation

reshleman
Copy link
Contributor

Currently, Roo raises a generic RuntimeError with the message
"Couldn't find header row" when a matching header row is not found.

This behavior makes it difficult to rescue from the most specific error
desired, because RuntimeError is the default error class for raise.

Additionally, because Roo does not have its own library-level error
class from which all errors inherit, it is difficult to differentiate
Roo library errors from errors caused by the developer / user.

From the Ruby documentation:

It is recommended that a library should have one subclass of
StandardError or RuntimeError and have specific exception types
inherit from it. This allows the user to rescue a generic exception
type to catch all exceptions the library may raise even if future
versions of the library add new exception subclasses.

This commit adds a generic Roo::Error class which inherits from
StandardError and a specific Roo::HeaderRowNotFound class that is
raised when a matching header row could not be found.

In the future, additional subclasses of Roo::Error can be added in
order to allow users to more easily rescue the correct errors.

Note: This is not a backwards-compatible change. Users who currently
rescue RuntimeError for a missing header row will now see an error
raised.

* Test 1 case with matching row and 1 without a matching row
Currently, Roo raises a generic `RuntimeError` with the message
"Couldn't find header row" when a matching header row is not found.

This behavior makes it difficult to rescue from the most specific error
desired, because `RuntimeError` is the default error class for `raise`.

Additionally, because Roo does not have its own library-level error
class from which all errors inherit, it is difficult to differentiate
Roo library errors from errors caused by the developer / user.

From the [Ruby documentation][1]:

> It is recommended that a library should have one subclass of
> `StandardError` or `RuntimeError` and have specific exception types
> inherit from it. This allows the user to rescue a generic exception
> type to catch all exceptions the library may raise even if future
> versions of the library add new exception subclasses.

This commit adds a generic `Roo::Error` class which inherits from
`StandardError` and a specific `Roo::HeaderRowNotFound` class that is
raised when a matching header row could not be found.

In the future, additional subclasses of `Roo::Error` can be added in
order to allow users to more easily rescue the correct errors.

Note: This is *not* a backwards-compatible change. Users who currently
rescue `RuntimeError` for a missing header row will now see an error
raised.

[1]: http://ruby-doc.org/core-2.2.2/Exception.html
@simonoff
Copy link
Member

I like it!

simonoff added a commit that referenced this pull request Aug 13, 2015
Missing Header Raises `Roo::HeaderRowNotFoundError`
@simonoff simonoff merged commit 4d747d9 into roo-rb:master Aug 13, 2015
@reshleman reshleman deleted the re-raise-header-error branch August 13, 2015 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants