Skip to content

Abstract KV layer options to eliminate dynamic casts to FDBTransaction #119

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Clcanny
Copy link
Contributor

@Clcanny Clcanny commented Mar 6, 2025

The use of dynamic_cast can bypass the common functionality defined by the interface classes (IReadOnlyTransaction and IReadWriteTransaction) and instead rely on specific implementation details. This might not be ideal, as it reduces abstraction and increases coupling to a particular implementation. To address this, I attempted to define common option-setting functions.

I considered two approaches:

  1. Providing a universal function such as setOption(TransactionOption option, std::string_view value) that can handle all options.
  2. Using separate, specific functions like enableStaleRead and setPriority.

I chose the second approach because it assumes there are not many options to set, and this method is clearer and type-safe compared to passing raw value strings.

Thank you for taking the time to review. Feel free to close the MR if it is too small or not helpful.

@Clcanny Clcanny force-pushed the remove-dynamic-cast branch 5 times, most recently from 78cd054 to 92fa470 Compare March 6, 2025 16:57
@Clcanny Clcanny force-pushed the remove-dynamic-cast branch from 92fa470 to 47a3ed4 Compare March 6, 2025 17:03
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.

1 participant