rippled
rippled copied to clipboard
XRPLF
Rename SHAMapStoreImp::stopping() to SHAMapStoreImp::handleHealth()
Context of Change
A recent change modified the name of a member function from SHAMapStoreImp::health() to SHAMapStoreImp::stopping(). Personally, I find the name change to stopping() misleads me when I read the source code. The function stopping() performs the job of looking at the health of rippled and sleeping the thread if rippled is not sufficiently healthy.
I would never expect a function named stopping() that returns a bool to sleep the thread.
This pull request renames that function to SHAMapStoreImp::handleHealth(). To assist with readability, handleHealth() returns an enum of either stopping or keepGoing. If handleHealth() returns stopping, then rippled is attempting to shut down and appropriate actions should be taken.
Type of Change
- [x] Refactor (non-breaking change that only restructures code)
@nbougalis, I disagree that the function's primary purpose is to determine whether we should stop. I believe the function's primary purpose is to sleep the thread if rippled is currently unhealthy. The thread may be awakened because the server is stopping, but that is not the primary point of the function. Expand the diff to show the internals of the function so you can see what's going on.
I'm not arguing that handleHealth is the best name. But neither healthy nor stopping represent the intent behind the function as I currently understand it. As it stands the return value from the function is a side effect of the primary purpose of the function.
@scottschurr I'd love to have the word wait in the name somewhere - or something that implies that this function can sleep.
@scottschurr I'd love to have the word
waitin the name somewhere - or something that implies that this function can sleep.
or yield()?
Maybe waitForHealth()? If that has too many characters, possibly healthWait()?
@scottschurr I'm fine with either of those. It's hard to name this function because it does two things - it waits for a condition and it returns if we're stopping. I understand why it does that, but it makes it hard to put a name to it. Anyway, either of those names sounds good to me. Thanks!
@nbougalis, this pull request how has two (non-authorized) approvals, and I've squashed the commits. So I think this pull request is ready to go. I'll have to leave it to you to mark the pull request as passed, since I no longer have those permissions. Thanks.
Rebased to 1.10.0-b1. @nbougalis, I believe this pull request is ready to be marked as passed. Please let me know if it needs any changes. Thanks.