Review Request: Rewrite the anagram generation from scratch

[Anne-Marie Mahfouf]

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


Tested and works fine, patch looks fine and makes code easier to read indeed.

- Anne-Marie Mahfouf


On Jan. 20, 2012, 12:03 p.m., Laszlo Papp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103747/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2012, 12:03 p.m.)
> 
> 
> Review request for KDE Edu and Jeremy Paul Whiting.
> 
> 
> Description
> -------
> 
> The current way of generating an anagram from a QString coming from the
> dictionary is a bit hard to read, maintain and not completely working.
> 
> 1) If the generated anagram is equal to the original, the re-calculation does
> not work because the original object is not set back again; it starts operating
> on the already truncated word.
> 
> 2) Storing the letters in a QStringList is suboptimal.
> 
> 3) Variable namings, like "insaneData" for the generated anagram, or "objChunk"
> for the random index are not that readable like in this patch
> 
> 4) Splitting the original word with "" rule is hard to read, understand and
> error-prone. I had to write a small workhorse where I realized this operation
> actually prepends and appends a "" at the beginning and end. This is more than
> what we need on the other hand (ie. we just need the letters).
> 
> 5) objData.count() as the condition of the loop is not that good and easy to
> read than simple !letters.isEmpty(). Also with this patch, we can spare one
> "count" variable.
> 
> 6. We do not need for loop anymore, just while. It means less statement and
> variable (no need for dealing with a loop variable, like 'i' in this special
> case).
> 
> 7) The whole method is now shorter and cleaner. Thereby, easier to read,
> understand and maintain.
> 
> I was wondering whether to use do/while or recursive call, but I decided to keep
> do/while since we need to try, at least once, to generate and anagram; that is
> what do/while is was designed for. Also, we spare some initialization with
> do/while.
> 
> 
> Diffs
> -----
> 
>   src/engine/kanagramgame.cpp e7d3500 
> 
> Diff: http://git.reviewboard.kde.org/r/103747/diff/diff
> 
> 
> Testing
> -------
> 
> Testing: I have just tested (build and runtime) the patch on my Archlinux
> laptop, and seems to work fine without any regressions.
> 
> 
> Thanks,
> 
> Laszlo Papp
> 
>

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