Refactor ActionHelper and ActionMessage to expect success boolean explicitely #377

New Issue

muellerr · 2021-03-03T15:08:15+01:00

muellerr commented

2021-03-03 15:08:15 +01:00

Right now the ActionHelper finish function looks like this:

    void finish(MessageQueueId_t reportTo, ActionId_t commandId,
            ReturnValue_t result = HasReturnvaluesIF::RETURN_OK);

I think it would be better if it looks like this

    void finish(bool success, MessageQueueId_t reportTo, ActionId_t commandId,
            ReturnValue_t result = HasReturnvaluesIF::RETURN_OK);

This is because the underlying completion reply implementation looks like this

void ActionMessage::setCompletionReply(CommandMessage* message,
        ActionId_t fid, ReturnValue_t result) {
    if (result == HasReturnvaluesIF::RETURN_OK or result == HasActionsIF::EXECUTION_FINISHED) {
        message->setCommand(COMPLETION_SUCCESS);
    } else {
        message->setCommand(COMPLETION_FAILED);
    }
    message->setParameter(fid);
    message->setParameter2(result);
}

So whether finish is successfull or not is implicitely defined by the returnvalue, which is not good in my opinion. The developer should be forced to determine whether their returnvalues are equal to a completion success or failure. I think this would be better:

void ActionMessage::setCompletionReply(bool success, CommandMessage* message,
        ActionId_t fid, ReturnValue_t result) {
    if (success) {
        message->setCommand(COMPLETION_SUCCESS);
    }
    else {
        message->setCommand(COMPLETION_FAILED);
    }
    message->setParameter(fid);
    message->setParameter2(result);
}

Right now the ActionHelper finish function looks like this: ```cpp void finish(MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result = HasReturnvaluesIF::RETURN_OK); ``` I think it would be better if it looks like this ```cpp void finish(bool success, MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result = HasReturnvaluesIF::RETURN_OK); ``` This is because the underlying completion reply implementation looks like this ```cpp void ActionMessage::setCompletionReply(CommandMessage* message, ActionId_t fid, ReturnValue_t result) { if (result == HasReturnvaluesIF::RETURN_OK or result == HasActionsIF::EXECUTION_FINISHED) { message->setCommand(COMPLETION_SUCCESS); } else { message->setCommand(COMPLETION_FAILED); } message->setParameter(fid); message->setParameter2(result); } ``` So whether finish is successfull or not is implicitely defined by the returnvalue, which is not good in my opinion. The developer should be forced to determine whether their returnvalues are equal to a completion success or failure. I think this would be better: ```cpp void ActionMessage::setCompletionReply(bool success, CommandMessage* message, ActionId_t fid, ReturnValue_t result) { if (success) { message->setCommand(COMPLETION_SUCCESS); } else { message->setCommand(COMPLETION_FAILED); } message->setParameter(fid); message->setParameter2(result); } ```

muellerr added the

feature

API Change

labels 2021-03-03 15:08:22 +01:00

muellerr changed title from ~~REfactor ActionHelper and ActionMessage to expect success boolean explicitely~~ to Refactor ActionHelper and ActionMessage to expect success boolean explicitely

2021-03-03 15:08:44 +01:00

gaisser referenced this issue

2021-03-04 21:13:10 +01:00

Update Package #378

muellerr commented

2021-03-10 16:25:58 +01:00

Was merged with #378

muellerr closed this issue

2021-03-10 16:25:58 +01:00

Sign in to join this conversation.