D27166: Refactor converter runner

[Harald Sitter]

sitter added a comment.


  Please make a cpps for you headers and move the function implementations there.
  
  +1 to regex

INLINE COMMENTS

> converter_utilities.h:23
> +
> +double stringToDouble(const QStringRef &value, bool *ok, const QLocale &locale)
> +{

Perhaps I am missing something but it seems to me that all the functions in here really should be statics inside the runner.cpp, don't you think?

> converterrunner.cpp:64
>  {
> -    const QString term = context.query();
> -    if (term.size() < 2) {
> +    const QString &term = context.query();
> +    if (term.size() < 2 || !context.isValid()) {

Is there a particular reason you hold it as a reference? (also applies to a bunch of QStrings and QLists below). We generally do not hold Qt types as references unless there is a reason to I think.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27166

To: alex, broulik, ngraham, #plasma
Cc: sitter, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200205/9b15ecbb/attachment-0001.html>