Review Request 118068: Parley: Implement early stage training

[Andreas Cord-Landwehr]

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


Just made a code-review, still have to test the changes.


src/practice/abstractbackendmode.h
<https://git.reviewboard.kde.org/r/118068/#comment40162>

    The doxygen comment is for the following method, not for currentPreGradeForEntry.
    Also, please add doxygen documentation for currentPreGradeForEntry where also the meaning of "pregrade" is explained in short



src/practice/abstractbackendmode.h
<https://git.reviewboard.kde.org/r/118068/#comment40163>

    Since this is a base class for derivation, I would like to see a more detailed doxygen documentation of what this function does.



src/practice/abstractbackendmode.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40164>

    shouldn't his method be "const"?



src/practice/abstractbackendmode.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40165>

    shouldn't shis method also be "const"?



src/practice/abstractbackendmode.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40166>

    looks more like "TODO" to me or is this really a problem right now?



src/practice/conjugationbackendmode.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40168>

    method should be "const"



src/practice/conjugationbackendmode.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40167>

    This is iterating two times over conj. Instead, by using a const iterator for conj you only had to iterate once.



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40169>

    tailing whitespace



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40170>

    coding style: missing opening and closing curly braces



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40171>

    coding style: missing opening and closing curly braces



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40172>

    coding style: missing opening and closing curly braces



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40173>

    coding style: missing opening and closing curly braces



src/practice/entryfilter.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40174>

    tailing whitespace



src/practice/guifrontend.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40175>

    tailing whitespace



src/practice/sessionmanagerfixed.cpp
<https://git.reviewboard.kde.org/r/118068/#comment40176>

    I do not see which heap object is not correctly destroyed... can you be a bit more concrete about the mem-leak?


- Andreas Cord-Landwehr


On May 10, 2014, 5:59 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118068/
> -----------------------------------------------------------
> 
> (Updated May 10, 2014, 5:59 a.m.)
> 
> 
> Review request for KDE Edu, Amarvir Singh and Andreas Cord-Landwehr.
> 
> 
> Repository: parley
> 
> 
> Description
> -------
> 
> This patch implements what we discussed on the mailing list, namely the early training stages before waiting 1 day to train a word again makes sense. It works exactly like in the later stages except:
>  - The intervals are much shorter (3.5 minutes to 8 hours)
>  - The intervals are not configurable
>  - There is no nice animation of boxes/diamonds/whatever.
> 
> I opted to use the pregrade element in the updated kvtml libraries rather than the interval element, also like discussed on the mailing list.
> 
> I have used this myself in my own language training and it works well and with excellent results I may add.
> 
> Work that remains to be done is on this feature:
>  - Improve statistics. This should be done so that it can also handle double-directed training
>  - Possibly improve the visualization
> I don't think the lack of any of these two invalidates merging already.
> 
> 
> Diffs
> -----
> 
>   src/practice/abstractbackendmode.cpp 19277e1 
>   src/practice/abstractfrontend.h b41bdae 
>   src/practice/comparisonbackendmode.cpp 00488ff 
>   src/practice/abstractbackendmode.h b804540 
>   TODO-PREGRADE PRE-CREATION 
>   src/practice/conjugationbackendmode.h 243cf98 
>   src/practice/conjugationbackendmode.cpp fdc38f4 
>   src/practice/entryfilter.cpp d0d2e41 
>   src/practice/genderbackendmode.cpp 6f7e99e 
>   src/practice/guifrontend.h 2d2a12b 
>   src/practice/guifrontend.cpp e88ff29 
>   src/practice/practice_mainwindow.ui 0a4a9a7 
>   src/practice/practicestatemachine.cpp 35e08e8 
>   src/practice/sessionmanagerfixed.cpp 0ee0035 
>   src/practice/testentry.h 454621b 
>   src/practice/testentry.cpp 1aee0ef 
> 
> Diff: https://git.reviewboard.kde.org/r/118068/diff/
> 
> 
> Testing
> -------
> 
> Lots of testing, including my own language training.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140511/003effe1/attachment-0001.html>