Review Request 122771: method to ignore extra white spaces.

Inge Wallin inge at lysator.liu.se
Sun Mar 8 18:02:36 UTC 2015


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



src/practice/writtenpracticevalidator.h
<https://git.reviewboard.kde.org/r/122771/#comment53021>

    As I think we agreed about on irc, the purpose is not to remove all whitespaces, only unnecessary ones.  So perhaps we should try to find another name for this function.



src/practice/writtenpracticevalidator.cpp
<https://git.reviewboard.kde.org/r/122771/#comment53022>

    You don't have to create a new string. You can just use 'letter' in the foreach(). Also, 'letter' is a bad name because it's not just one letter, it can contain one or even several sentences. So 'word' is also a bad name.



src/practice/writtenpracticevalidator.cpp
<https://git.reviewboard.kde.org/r/122771/#comment53023>

    I'm not 100% sure but I think removing a character from a string that you are looping through using foreach() is dangerous. I would suggest to let the original string be as it is, and instead build a new one from the characters that you select.


Some more comments

- Inge Wallin


On March 2, 2015, 11:53 a.m., ankit wagadre wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122771/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 11:53 a.m.)
> 
> 
> Review request for KDE Edu and Inge Wallin.
> 
> 
> Repository: parley
> 
> 
> Description
> -------
> 
> Method to ignore extra white spaces.And some corrections.
> 
> 
> Diffs
> -----
> 
>   src/practice/writtenbackendmode.cpp 23b0260 
>   src/practice/writtenpracticevalidator.h ee99b25 
>   src/practice/writtenpracticevalidator.cpp 714ce01 
> 
> Diff: https://git.reviewboard.kde.org/r/122771/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> ankit wagadre
> 
>

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


More information about the kde-edu mailing list