Review Request 109773: New runner that translates words and sentences via Google Translate.

[David Baum]

> On April 8, 2013, 5:39 p.m., Aaron J. Seigo wrote:
> > runners/translator/translator.cpp, lines 116-117
> > <http://git.reviewboard.kde.org/r/109773/diff/5/?file=131524#file131524line116>
> >
> >     might be nice to pop a small comment here why this is required

Google responds with a JSON array like this: ["foo",,"bar"]. Unfortunately this is nod valid JSON, QJSON expects ["foo","","bar"].
I added a comment in the code.


> On April 8, 2013, 5:39 p.m., Aaron J. Seigo wrote:
> > runners/translator/translator.cpp, line 131
> > <http://git.reviewboard.kde.org/r/109773/diff/5/?file=131524#file131524line131>
> >
> >     l0 is not a great variable name. please name it something descriptive to its purpose.

You're right. Hope it's better now.


> On April 8, 2013, 5:39 p.m., Aaron J. Seigo wrote:
> > runners/translator/translator.cpp, lines 184-192
> > <http://git.reviewboard.kde.org/r/109773/diff/5/?file=131524#file131524line184>
> >
> >     i would suggest taking advantage of the fact that QMap is sorted. in the while loop, the code already does a check for i.key() == key, so one may as well do that from the start and do something like:
> >     
> >     QMapIterator<int, QPair<QString, double> > it(sentences);
> >     int currentKey = -1;
> >     double currentRel = 1;
> >     QString currentString;
> >     
> >     while (it.hasNext()) {
> >         pair = it.next();
> >     
> >         // we're on to another key, process previous results, if any
> >         if (currentKey != it.key()) {
> >     
> >             if (!combined.isEmpty()) {
> >                 Plasma::QueryMatch match(this);
> >                 .. setup match ..
> >                 combined.clear();
> >             }
> >     
> >             currentKey = it.key();
> >             currentRel = 1;
> >             currentString.clear();
> >         }
> >     
> >         currentString.append(' ').append(pair.first);
> >         currentRel *= pair.second;
> >     }    
> >     
> >     this woudl avoid the more expensive calls of uniqueKeys and find and make the code a lot easier to read. (assuming it does what i think it does from reading it :)

Nice! And yes, it does what you think it does. ;-)


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109773/#review30684
-----------------------------------------------------------


On April 15, 2013, 1:17 a.m., David Baum wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109773/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 1:17 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> The Runner uses the Google Translate Homepage and supports all languages provided by Google. It doesn't use the API, because it's not free of charge. 
> 
> 
> Diffs
> -----
> 
>   runners/CMakeLists.txt bb4b491b10e6fef8183a66f55f5d5832dd7bc41a 
>   runners/translator/CMakeLists.txt PRE-CREATION 
>   runners/translator/plasma-runner-translator.desktop PRE-CREATION 
>   runners/translator/translator.h PRE-CREATION 
>   runners/translator/translator.cpp PRE-CREATION 
>   runners/translator/translatorjob.h PRE-CREATION 
>   runners/translator/translatorjob.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109773/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Baum
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130414/008dc6e7/attachment-0001.html>