queue nullptr check in action helper #458

meierj · 2021-08-07T14:31:04+02:00

meierj commented

2021-08-07 14:31:04 +02:00

added nullptr check for queueToUse in ActionHelper

* added nullptr check for queueToUse in ActionHelper

meierj added 1 commit 2021-08-07 14:31:04 +02:00

queue nullptr check in action helper 20adc1c981

mohr reviewed 2021-08-09 12:04:36 +02:00

mohr left a comment

I would suggest removing setQueueToUse(MessageQueueIF* queue), when introducing the nullptr check as it becomes mostly useless.

I would suggest removing `setQueueToUse(MessageQueueIF* queue)`, when introducing the `nullptr` check as it becomes mostly useless.

action/ActionHelper.cpp Outdated

						
				@ -33,6 +33,17 @@ ReturnValue_t ActionHelper::initialize(MessageQueueIF* queueToUse_) {

				        setQueueToUse(queueToUse_);

mohr commented

2021-08-09 12:02:06 +02:00

As there is a check for nullptr in the following line, this if can be removed and setQueueToUse(queueToUse_); be moved to the end.

As there is a check for nullptr in the following line, this if can be removed and ` setQueueToUse(queueToUse_);` be moved to the end.

action/ActionHelper.cpp Outdated

						
				@ -33,6 +33,17 @@ ReturnValue_t ActionHelper::initialize(MessageQueueIF* queueToUse_) {

				        setQueueToUse(queueToUse_);

				    }

				    if(queueToUse == nullptr) {

mohr commented

2021-08-09 12:02:02 +02:00

If setQueueToUse(queueToUse_); is moved to below this check, queueToUse_ needs to be checked instead of queueToUse.

If `setQueueToUse(queueToUse_);` is moved to below this check, `queueToUse_` needs to be checked instead of `queueToUse`.

gaisser commented

2021-08-16 14:56:27 +02:00

queueToUse_ is allowed to be nullptr in ActionHelper::initialize(MessageQueueIF* queueToUse_) as queueToUse can be set in the constructor or by calling setQueueToUse(xxx) before initialize. Therefore, we need to check both as we have three cases:

queueToUse was set before
1. queueToUse_ can be nullptr and setQueueToUse(queueToUse_); should not be called
queueToUse was not set before
1. queueToUse_ is nullptr -> Error Message
2. queueToUse_ is not a nullptr -> call setQueueToUse(queueToUse_);

``queueToUse_`` is allowed to be nullptr in `` ActionHelper::initialize(MessageQueueIF* queueToUse_)`` as ``queueToUse`` can be set in the constructor or by calling ``setQueueToUse(xxx)`` before ``initialize``. Therefore, we need to check both as we have three cases: 1. ``queueToUse`` was set before 1. ``queueToUse_`` can be nullptr and `` setQueueToUse(queueToUse_); `` should not be called 2. ``queueToUse`` was not set before 1. ``queueToUse_`` is nullptr -> Error Message 2. ``queueToUse_`` is not a nullptr -> call `` setQueueToUse(queueToUse_); ``