Code review comment for lp://staging/~abreu-alexandre/webbrowser-app/intent

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > 279 + // Fallback to a noop
> > 280 + result = d->evaluate(
> > 281 +
> > IntentFilterPrivate::DEFAULT_PASS_THROUGH_FILTER
> > 282 + , intentDescription).toVariant().toMap();
> >
> > This looks like a rather complex (and expensive) way of transforming
> > intentDescription into a QVariantMap.
> > What about getting rid of the IntentUriDescription struct altogether, to
> > replace it everywhere with QVariantMap?
>
> The 'expensive' factor is very relative to the usage context ... but in this
> case it not that, ... it has nothing to do w/ intentDescription, but w/ the
> result
> of QJSValue -> QVAriantMap ...

Sure, the cost is probably overall negligible. Still, in this case d->evaluate(…) is called only to turn an IntentUriDescription into a QVariantMap (there’s no need to use a temporary QJSValue for that), and this feels like killing a fly with a sledgehammer to me. But I’m fine with it either way, after all, it works.

« Back to merge proposal