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

Refactor several items #78

Merged
merged 4 commits into from
Apr 17, 2024
Merged

Refactor several items #78

merged 4 commits into from
Apr 17, 2024

Conversation

bgilbert
Copy link
Member

Only wrap FFM exceptions if necessary. Exceptions from FFM invocations are generally RuntimeExceptions, so we can throw them as-is. Any others we need to wrap. In the latter case, wrap with an IllegalArgumentException, since that's a better guess at the cause than AssertionError.

Push locking down into OpenSlideFFM. Implement read-write locking for use-after-free checks in a single place in OpenSlideFFM.Ref, which is logically a better place for it and simplifies the higher-level code.

Deprecate the now-unused, but public, zero-argument OpenSlideDisposedException constructor.

Use AutoCloseable wrapper class for OpenSlide error checks. Rather than manually calling checkError() after every set of OpenSlide calls, introduce a wrapper class which holds the OpenSlideRef and can be used as a resource in a try-with-resources block. When exiting the block, its close() method can then run the error check.

Deprecate OpenSlide.dispose(). close() is more idiomatic because of Closeable and has existed since 2010.

Exceptions from FFM invocations are generally RuntimeExceptions, so we can
throw them as-is.  Any others we need to wrap.  In the latter case, wrap
with an IllegalArgumentException, since that's a better guess at the cause
than AssertionError.

Signed-off-by: Benjamin Gilbert <[email protected]>
@openslide-bot
Copy link

openslide-bot commented Apr 17, 2024

DCO signed off ✔️

All commits have been signed off. You have certified to the terms of the Developer Certificate of Origin, version 1.1. In particular, you certify that this contribution has not been developed using information obtained under a non-disclosure agreement or other license terms that forbid you from contributing it under the GNU Lesser General Public License, version 2.1.

Implement read-write locking for use-after-free checks in a single place
in OpenSlideFFM.Ref.  This is logically a better place for it and
simplifies the higher-level code.

Deprecate the now-unused, but public, zero-argument
OpenSlideDisposedException constructor.

OpenSlideFFM doesn't use the scoped locks inside the try-with-resources
blocks, and the compiler wants to warn about it.  Rather than add no-op
lock references or suppression boilerplate in every method, suppress try
warnings for the whole class.

Signed-off-by: Benjamin Gilbert <[email protected]>
Rather than manually calling checkError() after every set of OpenSlide
calls, introduce a wrapper class which holds the OpenSlideRef and can be
used as a resource in a try-with-resources block.  When exiting the block,
its close() method can then run the error check.  This effectively uses
AutoCloseable as a Python context manager, which is a bit unusual but
doesn't violate the AutoCloseable API contract.

Signed-off-by: Benjamin Gilbert <[email protected]>
close() is more idiomatic because of Closeable and has existed since 2010.

Signed-off-by: Benjamin Gilbert <[email protected]>
@bgilbert bgilbert merged commit e32fe85 into openslide:main Apr 17, 2024
5 checks passed
@bgilbert bgilbert deleted the refactor branch April 17, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants