lordgamez commented on a change in pull request #1281: URL: https://github.com/apache/nifi-minifi-cpp/pull/1281#discussion_r831916481
########## File path: libminifi/include/properties/ConfigurationReader.h ########## @@ -0,0 +1,32 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <optional> +#include <string> + +namespace org::apache::nifi::minifi { + +class ConfigurationReader { + public: + virtual bool get(const std::string& key, std::string& value) const = 0; + virtual bool get(const std::string& key, const std::string& alternate_key, std::string& value) const = 0; + virtual std::optional<std::string> get(const std::string& key) const = 0; +}; Review comment: I was referring to classes like the `AgentIdentificationProvider` or the `StateMonitor` where those classes are also implemented as separate interfaces to be later set in one of the agent information nodes. I think these adhere more to the interface segregation principle and especially in case of the FlowController where there are already way too many responsibilities they could be later extracted to separate classes and composed instead of inheriting from them. Also `AgentIdentificationProvider` is already one interface which is extracted and implemented by the `Configure` class. **BUT** I rechecked this case specifically and as you mentioned we only need a single overload of the get functions and I'm not sure if this "read-only configuration" interface will be reused in the future anywhere else, so after these considerations I replaced it with a single std::function in c2544df895ed1fac5ba6d5b50e27ecea4e073c3e -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
