bakaid commented on a change in pull request #605: MINIFICPP-550 - Implement
RocksDB controller service and component st…
URL: https://github.com/apache/nifi-minifi-cpp/pull/605#discussion_r389753888
##########
File path: libminifi/include/core/ProcessContext.h
##########
@@ -193,6 +216,61 @@ class ProcessContext : public
controller::ControllerServiceLookup, public core::
return controller_service_provider_->getControllerServiceName(identifier);
}
+ static constexpr char const* DefaultStateManagerProviderName =
"defaultstatemanagerprovider";
Review comment:
> default- ok
> state- stateless is generally better, but ok
This is a PR about creating a centralized state storage mechanism for
processors that require state, instead of all of them implementing individual
state files. "Stateless is generally better" is a mind-bogglingly meaningless
comment here.
> manager- meaningless. Only suggests doing something, so it should probably
be a verb. Having a noun in place of a verb usually means trying to use a class
where a function would be better.
StateManager comes from NiFi, and it is a perfectly meaningful name: it is
an object that helps manage the state of a single CoreComponent. It cannot be a
function as it contains some caching and validation logic that requires it in
itself to be stateful.
> provider- again, a class instead of a function. This has a meaning though.
CoreComponentStateManagerProvider needs to exist as interface, which is used
by the ClassLoader and we access different implementations of it through that
interface.
> name- ok
>
> I'm not saying that your code is bad, just that this is a red flag that
suggests a design mistake that may or may not be in your changes.
I'm not saying your comment is useless, but it could be more useful if you
tried to interpret the name in the context of the PR, instead of in itself.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
