Review Request 127505: Parley: Randomize order of letters in Mixed Letters Practice

Julian Helfferich julian.helfferich at googlemail.com
Sun Mar 27 19:32:35 UTC 2016



> On March 27, 2016, 9:49 a.m., Andreas Cord-Landwehr wrote:
> > The change looks good to me. Also considering the commit message by Inge (btw in Sweden "Inge" is a male name AFAIK ;), this pretty much looks like a KF5 porting bug to me.
> > Please wait a few days if you get a reply, else consider this as a "Ship-It". We should get this patch into the coming release.

I apologize if I mixed up Inge's gender. In Germany it's an exclusively female name. No offence intended.

I assume the reason for removing KRandomSequence was that it got instantiated in setSolution() and thus once for every single word. This is of course very inefficient.


- Julian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127505/#review94041
-----------------------------------------------------------


On March 27, 2016, 2:13 a.m., Julian Helfferich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127505/
> -----------------------------------------------------------
> 
> (Updated March 27, 2016, 2:13 a.m.)
> 
> 
> Review request for KDE Edu and Inge Wallin.
> 
> 
> Bugs: 360239
>     http://bugs.kde.org/show_bug.cgi?id=360239
> 
> 
> Repository: parley
> 
> 
> Description
> -------
> 
> The order of letters in the Mixed Letter Practice is currently not randomized. Instead, the letters are only shifted vertically (see bug 360239).
> 
> Now, I use KRandomSequence::randomize() to shuffle the list of characters comprising the solution. The KRandomSequence is a member of MixedLettersModeWidget. Thus, it gets instantiated each time the Mixed Letters Practice is started. I also use KRandomSequence::getInt() instead of KRandom::random() for consistency.
> 
> I have added Inge Wallin to the reviewers, because she replaced KRandomSequence with KRandom about a year ago. Is there a good reason not to use KRandomSequence?
> 
> 
> Diffs
> -----
> 
>   src/practice/mixedlettersmodewidget.h f2acaa1 
>   src/practice/mixedlettersmodewidget.cpp adc53cd 
> 
> Diff: https://git.reviewboard.kde.org/r/127505/diff/
> 
> 
> Testing
> -------
> 
> Build and tested on Kubuntu 15.10:
> 
> Practiced vocabulary using Mixed Letter Practice and confirmed that letters are randomized.
> 
> 
> Thanks,
> 
> Julian Helfferich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20160327/02b6afd5/attachment.html>


More information about the kde-edu mailing list