[Development] QOptional

Robert Knight robertknight at gmail.com
Thu Aug 21 15:07:09 CEST 2014


> Bot classes do NOT allow any default conversions (neither bool nor
> value nor string) which made their usage a bit more verbose but
> improved readability and safety a lot.

My experience has been similar. I took the std::optional proposal as a
starting point and stripped it back to just the basics.
Aside from the removal of implicit conversions the other major change
was replacing the std::logic_error exception when trying to access
an unset Option with a call to qFatal(), since the Qt event loop loses
the original stack trace if it catches and rethrows an exception - I
don't suppose
anyone knows of a way around that?

On 21 August 2014 13:41, Julien Blanc <julien.blanc at nmc-company.com> wrote:
>
> On 21/08/2014 13:01, Poenitz Andre wrote:
>> Julien Blanc wrote:
>>> There are two strong arguments in favor of optional :
>>> - real life tends to show that the optional &ok parameter is often
>>> missed in context it should not, leading to hard to find bugs.
>>> - optional is much more friendly to static verification than &ok (dependancy between variables is a mess to check).
>>>
>> I am actually not quite sure what you mean with the second item.
>
> I’m talking about what tools like coverity / polyspace can verify on the
> code. Checking that an optional is not used when it is not tested for
> nullness is doable (it is a lot like static checking against null
> pointers dereference), whereas checking for another variable is beyond
> what current static checkers can do afaik.
>
>>
>> Something like the following?
>>
>> struct S { int m_a, int m_b, int m_c);
>>
>> // "Tradtional, wrong result"
>> bool S::parse(QString a, QString b, QString c)
>> {
>>      bool ok;
>>      m_a = a.toInt(&ok);
>>      m_b = b.toInt(&ok); // Oops, overwrites first ok value.
>>      m_c = c.toInt(&ok);
>>      return ok;
>> }
>
> I wasn’t thinking about this one, but that’s an issue, yes.
>
>>
>> vs
>>
>> // "Tradtional, right result"
>> bool S::parse(QString a, QString b, QString c)
>> {
>>      bool oa, ob, oc;
>>      m_a = a.toInt(&oa);
>>      m_b = b.toInt(&ob);
>>      m_c = c.toInt(&oc);
>>      return oa && ob && oc;
>> }
>
> I would not recommend writing code like that (although i think it’s
> widespread). I would expect that parse, when failing, does not alter the
> object at all, so that it stays in a corerent state (aka
> “strong-guarantee”). This is not the case with such implementation.
>
> I would write it that way :
>
>     bool S::parse(QString a, QString b, QString c)
>     {
>          int tmpa, tmpb, tmpc;
>          bool oa, ob, oc;
>          tmpa = a.toInt(&oa);
>          tmpb = b.toInt(&ob);
>          tmpc = c.toInt(&oc);
>          if(oa && ob && oc)
>          {
>               m_a = tmpa;
>               m_b = tmpb;
>               m_c = tmpc;
>               return true;
>          }
>          return false;
>      }
>
>
>>
>> vs
>>
>> // "Optional"
>> bool S::parse(QString a, QString b, QString c)
>> {
>>      auto oa = a.toInt();
>>      auto ob = b.toInt();
>>      auto oc = c.toInt();
>>      m_a = *oa;
>>      m_b = *ob;
>>      m_c = *oc;
>>      return oa && ob && oc;
>
>
> This one is obviously wrong and would trigger an undefined behaviour if
> any string is not convertible to int. The good thing is that I expect
> smart static verification tools to tell you this. To fix it, I would
> write it that way :
>
>      bool S::parse(QString a, QString b, QString c)
>     {
>          auto oa = a.toInt();
>          auto ob = b.toInt();
>          auto oc = c.toInt();
>          if(oa && ob && oc)
>          {
>               m_a = *oa;
>               m_b = *ob;
>               m_c = *oc;
>               return true;
>          }
>          return false;
>      }
>
> Note that we gained strong guarantee in the process without even aiming
> for it, at no line of code cost.
>
>>
>> vs
>>
>> // "isInt"
>> bool S::parse(QString a, QString b, QString c)
>> {
>>      bool oa = a.isInt(&m_a);
>>      bool ob = b.isInt(&m_b);
>>      bool oc = c.isInt(&m_c);
>>      return oa && ob && oc;
>> }
>>
>> vs
>>
>> // "something else"
>> bool S::parse(QString a, QString b, QString c)
>> {
>>      return a.isInt(&m_a) && b.isInt(&m_b) && c.isInt(&m_c);
>> }
>
> Those versions have the same “no strong-guarantee” problem.
>
>> If so, the "Optional" variant doesn't look much simpler than any
>> of the others, even less so when one doesn't accept the use of
>> 'auto' there.
>
> I never advocated that Optional makes for shorter code :) (yup, I know
> others have, but i disagree with that statement). However, i think
> Optional is a mean for *better* code. Your examples confort me in this
> thinking : with Optional, strong-guarantee is achieved within the same
> code amount. Without, it requires at least twice as more code than the
> unsafe version.
>
> Regards,
>
> Julien
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development



More information about the Development mailing list