hunyadi-dev commented on a change in pull request #797:
URL: https://github.com/apache/nifi-minifi-cpp/pull/797#discussion_r431226910
##########
File path: libminifi/include/core/ConfigurableComponent.h
##########
@@ -216,16 +216,20 @@ bool ConfigurableComponent::getProperty(const std::string
name, T &value) const{
auto &&it = properties_.find(name);
if (it != properties_.end()) {
- Property item = it->second;
- value = static_cast<T>(item.getValue());
- if (item.getValue().getValue() != nullptr){
- logger_->log_debug("Component %s property name %s value %s", name,
item.getName(), item.getValue().to_string());
- return true;
- }
- else{
+ const Property& item = it->second;
+ if (item.getValue().getValue() == nullptr) {
+ // empty value
+ if (item.getRequired()) {
+ logger_->log_debug("Component %s required property %s is empty",
name, item.getName());
+ throw utils::RequiredPropertyMissingException("Required property is
empty: " + item.getName());
Review comment:
I am not saying that the logic is bad, just that this requires knowledge
of the implementation details of `getProperty`. My expectation on calling a
function of `bool getProperty(std::string)` is that I get a false returned if
there is an error, and I might miss the fact that this throws. It would either
be a good idea to show that this throws-if-required-missing in the name of the
function or move the whole is-required check up to the scope of the caller.
----------------------------------------------------------------
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:
[email protected]
