Make the comparison operators == / != of ito::RetVal const if the AddInInterface is changed the next time

Create issue
Issue #137 new
M. Gronle created an issue

The operators ito::RetVal::operator== and ito::RetVal::operator!= can be const, to allow a comparison for const ito::RetVal, too.

Change this the next time, the addInInterface is increased.

Comments (7)

  1. Oliver Schwanke

    is it possible to also make the constructor argument explicit(so it takes int or ito::retVal enums)?

    enums only would be awesome but i think lots of plugins will have to be adapted then…

    I have seen code that just returns false on error, which implicitly casts to an ito::RetVal(ito::retOk), which is most likely not as intended by the programmer.

    check Qitom/organitzer/widgetWrapper.cpp::932ff

  2. M. Gronle reporter

    I like the idea with the explicit constructors. If this explicit keyword does not break the binary compatibility, we can also do it right now. Else I prefer to wait for a major version change of itom and the underlying AddInInterface. However, I would not accept explicit integer values instead of the enumeration again. I don’t know why we have that kind of constructor right now; I would delete this one in the future 😉 . I don’t think that many plugins have used an integer instead of retOk, retWarning, retError and if so, this should be changed (I will check this at least for the open source plugins).

    Your mentioned bug in widgetWrapper is already fixed.

  3. M. Gronle reporter

    I just tried it out and I guess that it is not that easy:

    If we would write:

    class RetVal
    {
    RetVal() {...}
    explicit RetVal(ito::tRetValue retValue) ...
    }
    

    an initialization like

    ito::RetVal r = ito::retError
    

    does not work anymore, since this requires a copy initilization, which is not covered by the explicit keyword. It must be implicit.

    See also: https://en.cppreference.com/w/cpp/language/copy_initialization and the examples in https://en.cppreference.com/w/cpp/language/explicit

    However, removing the RetVal(int retValue) constructor is still a good idea.

  4. Oliver Schwanke

    Hi Marc,

    i’m not sure whether it breaks binary compatibility.

    But some plugins(at least of ours) will not compile anymore.

    This is because “no nice code” is pretty usual.

    As a first approach, maybe it would be just possible to use

    class RetVal {

    ...

    RetVal(bool) = delete;

    ...}

    to find the most probable bugs.

    The = delete syntax is cpp11 and should work on Tobi’s PC(should be switched on in the CMakeLsits explicitly for some veryvery old gcc...)

    So, i’m not actually sure on how to introduce this sort of changes to itom.

    I would suggest some sort of compiler switch, that switches compilation mode to something more strict,

    so the compiler complains and developers are guided to produce some nicer code.

    But you can still compile old code if you really wanted to.

  5. Log in to comment